Re: [RFA] Don't kill inferior if there's a typo in the specified port.

[Doug Evans <dje@google.com>]

[-- Attachment #1: Type: text/plain, Size: 2757 bytes --]

On Mon, Apr 13, 2009 at 11:49 AM, Doug Evans <dje@google.com> wrote:
> On Sat, Apr 11, 2009 at 8:04 AM, Pedro Alves <pedro@codesourcery.com> wrote:
>>> It also moves the status message to the callback;
>>> it's only called when exiting gdb (and if that changes one can split
>>> the function into silent/verbose versions).
>>> An alternative is something like this:
>>>
>>> -      fprintf (stderr, "Killing all inferiors\n");
>>> +      fprintf (stderr, "Detaching or killing all inferiors\n");
>>>
>>> but will that be confusing?
>>
>> Perhaps:
>>
>>  "Detaching from inferiors we had attached to, and killing all others\n"
>>
>> dunno, don't want to start a bikeshed-ish discussion on that.
>>
>>> OTOH, if gdbserver ever gets used with 10's or 100's of processes,
>>> exiting with one line per process may be a bit much.
>>> OTOOH, the user might like to know what got detached and what got killed.
>>> I don't know, but I can change the patch to do whatever y'all prefer.
>>
>> Yeah.  If this is a real problem, then we could output something like,
>>
>>  Killing process(es) PID1, PID2, PID3, PID4, PIDnn.
>>  Detaching process(es) PID1, PID2, PID3, PID4, PIDnn.
>>
>> This should mitigate the problem, although if you're really attached to
>> 100's of processes, it will still print a lot.
>>
>> The other thing that crossed my mind was that if you had a bunch
>> of processes, the real error message that let to gdbserver bailing out
>> would be buried in the output many lines before all those
>> "Killing/detaching process X".  The only version that doesn't
>> have this problem is the one that doesn't include any detail, like
>> the first option.
>
> One reason why I like printing which processes were detached from and
> which were killed is: What happens if the process is gone shortly
> after gdbserver exits?  Did gdbserver kill it, or did the detach screw
> up, or did the program crash on its own shortly after gdbserver
> detached?  Tracking this down will be a pain without this info.  One
> could add a verbose option to print such info I guess, but it might
> cut down on support costs if this info was always printed.
>
>>
>> Anyway, I've no real inclination for any of these possible
>> outputs.
>>
>>> Ok to check in?
>>
>> This is fine with me.  Let's give it a few days in case Daniel or
>> others want to comment.
>>
>> Thanks for documenting detach_or_kill_inferior_callback, btw ;-).
>>
>> --
>> Pedro Alves
>>
>

Ok to check this in?
It does

Killing process(es) PID1, PID2, PID3, PID4, PIDnn.
Detaching process(es) PID1, PID2, PID3, PID4, PIDnn.

though I removed the commas for simplicity.
[I'm proactively avoiding some of the issues being raised. :-)]

[-- Attachment #2: gdb-090430-gdbserver-attached-2.patch.txt --]
[-- Type: text/plain, Size: 5992 bytes --]

2009-04-30  Doug Evans  <dje@google.com>

	* inferiors.c (started_inferior_callback): New function.
	(attached_inferior_callback): New function.
	(have_started_inferiors_p, have_attached_inferiors_p): New functions.
	* server.c (print_started_pid, print_attached_pid): New functions.
	(detach_or_kill_for_exit): New function.
	(main): Call it instead of for_each_inferior (kill_inferior_callback).
	* server.h (have_started_inferiors_p): Declare.
	(have_attached_inferiors_p): Declare.

Index: inferiors.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/inferiors.c,v
retrieving revision 1.21
diff -u -p -r1.21 inferiors.c
--- inferiors.c	30 Apr 2009 18:35:55 -0000	1.21
+++ inferiors.c	30 Apr 2009 21:03:31 -0000
@@ -127,6 +127,8 @@ add_inferior_to_list (struct inferior_li
   list->tail = new_inferior;
 }
 
+/* Invoke ACTION for each inferior in LIST.  */
+
 void
 for_each_inferior (struct inferior_list *list,
 		   void (*action) (struct inferior_list_entry *))
@@ -447,6 +449,46 @@ find_process_pid (int pid)
     find_inferior_id (&all_processes, pid_to_ptid (pid));
 }
 
+/* Return non-zero if INF, a struct process_info, was started by us,
+   i.e. not attached to.  */
+
+static int
+started_inferior_callback (struct inferior_list_entry *entry, void *args)
+{
+  struct process_info *process = (struct process_info *) entry;
+
+  return ! process->attached;
+}
+
+/* Return non-zero if there are any inferiors that we have created
+   (as opposed to attached-to).  */
+
+int
+have_started_inferiors_p (void)
+{
+  return (find_inferior (&all_processes, started_inferior_callback, NULL)
+	  != NULL);
+}
+
+/* Return non-zero if INF, a struct process_info, was attached to.  */
+
+static int
+attached_inferior_callback (struct inferior_list_entry *entry, void *args)
+{
+  struct process_info *process = (struct process_info *) entry;
+
+  return process->attached;
+}
+
+/* Return non-zero if there are any inferiors that we have attached to.  */
+
+int
+have_attached_inferiors_p (void)
+{
+  return (find_inferior (&all_processes, attached_inferior_callback, NULL)
+	  != NULL);
+}
+
 struct process_info *
 get_thread_process (struct thread_info *thread)
 {
Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.95
diff -u -p -r1.95 server.c
--- server.c	1 Apr 2009 22:50:24 -0000	1.95
+++ server.c	30 Apr 2009 21:03:31 -0000
@@ -1808,6 +1808,11 @@ kill_inferior_callback (struct inferior_
   discard_queued_stop_replies (pid);
 }
 
+/* Callback for for_each_inferior to detach or kill the inferior,
+   depending on whether we attached to it or not.
+   We inform the user whether we're detaching or killing the process
+   as this is only called when gdbserver is about to exit.  */
+
 static void
 detach_or_kill_inferior_callback (struct inferior_list_entry *entry)
 {
@@ -1822,6 +1827,65 @@ detach_or_kill_inferior_callback (struct
   discard_queued_stop_replies (pid);
 }
 
+/* for_each_inferior callback for detach_or_kill_for_exit to print
+   the pids of started inferiors.  */
+
+static void
+print_started_pid (struct inferior_list_entry *entry)
+{
+  struct process_info *process = (struct process_info *) entry;
+
+  if (! process->attached)
+    {
+      int pid = ptid_get_pid (process->head.id);
+      fprintf (stderr, " %d", pid);
+    }
+}
+
+/* for_each_inferior callback for detach_or_kill_for_exit to print
+   the pids of attached inferiors.  */
+
+static void
+print_attached_pid (struct inferior_list_entry *entry)
+{
+  struct process_info *process = (struct process_info *) entry;
+
+  if (process->attached)
+    {
+      int pid = ptid_get_pid (process->head.id);
+      fprintf (stderr, " %d", pid);
+    }
+}
+
+/* Call this when exiting gdbserver with possible inferiors that need
+   to be killed or detached from.  */
+
+static void
+detach_or_kill_for_exit (void)
+{
+  /* First print a list of the inferiors we will be killing/detaching.
+     This is to assist the user, for example, in case the inferior unexpectedly
+     dies after we exit: did we screw up or did the inferior exit on its own?
+     Having this info will save some head-scratching.  */
+
+  if (have_started_inferiors_p ())
+    {
+      fprintf (stderr, "Killing process(es):");
+      for_each_inferior (&all_processes, print_started_pid);
+      fprintf (stderr, "\n");
+    }
+  if (have_attached_inferiors_p ())
+    {
+      fprintf (stderr, "Detaching process(es):");
+      for_each_inferior (&all_processes, print_attached_pid);
+      fprintf (stderr, "\n");
+    }
+
+  /* Now we can kill or detach the inferiors.  */
+
+  for_each_inferior (&all_processes, detach_or_kill_inferior_callback);
+}
+
 static void
 join_inferiors_callback (struct inferior_list_entry *entry)
 {
@@ -2015,9 +2079,7 @@ main (int argc, char *argv[])
 
   if (setjmp (toplevel))
     {
-      fprintf (stderr, "Killing all inferiors\n");
-      for_each_inferior (&all_processes,
-			 kill_inferior_callback);
+      detach_or_kill_for_exit ();
       exit (1);
     }
 
@@ -2062,8 +2124,7 @@ main (int argc, char *argv[])
 
       if (exit_requested)
 	{
-	  for_each_inferior (&all_processes,
-			     detach_or_kill_inferior_callback);
+	  detach_or_kill_for_exit ();
 	  exit (0);
 	}
       else
Index: server.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.h,v
retrieving revision 1.56
diff -u -p -r1.56 server.h
--- server.h	3 Apr 2009 20:15:51 -0000	1.56
+++ server.h	30 Apr 2009 21:03:31 -0000
@@ -236,6 +236,8 @@ void add_thread (ptid_t ptid, void *targ
 struct process_info *add_process (int pid, int attached);
 void remove_process (struct process_info *process);
 struct process_info *find_process_pid (int pid);
+int have_started_inferiors_p (void);
+int have_attached_inferiors_p (void);
 
 struct thread_info *find_thread_pid (ptid_t ptid);