Make async_disconnect try harder to shut down target

Joseph S. Myers joseph@codesourcery.com
Wed Sep 21 03:55:00 GMT 2011


Suppose GDB is being used with "targer remote |" and receives a SIGHUP, 
the event loop starts the async_disconnect processing and, before this 
finishes, GDB receives a SIGINT.  The SIGINT sets quit_flag and some code 
involved in the shutdown then checks this, via the QUIT macro, and throws 
an exception caught in catch_errors called by async_disconnect.  This 
happens before the remote target code has had time to use the remote 
protocol to tell the process run with "target remote |" to shut down, and 
before the closing of the remote target which would kill the process on 
the other end of the pipe.  So that process is left running when GDB 
quits.

Exactly this race occurs when the GDB testsuite is run with a board file 
using "target remote |".  DejaGnu's remote.exp:standard_close starts a 
shell in the background which sends the SIGINT, following this by closing 
the terminal in which GDB was running, which sends the SIGHUP.  Depending 
on how long it takes the shell to start up and send the signal, the 
signals can arrive in the sequence I listed; I have seen hundreds of qemu 
processes left behind by a test run from this cause.

This patch addresses the problem by arranging for pop_all_targets to be 
called in the event of an exception from quit_cover.  It appears to fix 
the problem for a GDB testsuite run (i.e. stops it leaving qemu processes 
behind), although it's possible other such races remain.  OK to commit?

2011-09-20  Joseph Myers  <joseph@codesourcery.com>

	* event-top.c (async_disconnect): If an exception is thrown from
	quit_cover, call pop_all_targets.  Use TRY_CATCH instead of
	catch_errors.
	* top.c (quit_cover): Return void and take no arguments.
	* top.h (quit_cover): Update prototype.

Index: event-top.c
===================================================================
RCS file: /cvs/src/src/gdb/event-top.c,v
retrieving revision 1.83
diff -u -p -r1.83 event-top.c
--- event-top.c	6 Sep 2011 14:48:59 -0000	1.83
+++ event-top.c	20 Sep 2011 15:29:37 -0000
@@ -870,9 +870,25 @@ handle_sighup (int sig)
 static void
 async_disconnect (gdb_client_data arg)
 {
-  catch_errors (quit_cover, NULL,
-		"Could not kill the program being debugged",
-		RETURN_MASK_ALL);
+  volatile struct gdb_exception exception;
+
+  TRY_CATCH (exception, RETURN_MASK_ALL)
+    {
+      quit_cover ();
+    }
+
+  if (exception.reason < 0)
+    {
+      fputs_filtered ("Could not kill the program being debugged",
+		      gdb_stderr);
+      exception_print (gdb_stderr, exception);
+    }
+
+  TRY_CATCH (exception, RETURN_MASK_ALL)
+    {
+      pop_all_targets (1);
+    }
+
   signal (SIGHUP, SIG_DFL);	/*FIXME: ???????????  */
   raise (SIGHUP);
 }
Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.207
diff -u -p -r1.207 top.c
--- top.c	6 Sep 2011 14:48:59 -0000	1.207
+++ top.c	20 Sep 2011 15:29:37 -0000
@@ -288,14 +288,13 @@ void (*deprecated_context_hook) (int id)
 /* NOTE 1999-04-29: This function will be static again, once we modify
    gdb to use the event loop as the default command loop and we merge
    event-top.c into this file, top.c.  */
-/* static */ int
-quit_cover (void *s)
+/* static */ void
+quit_cover (void)
 {
   caution = 0;			/* Throw caution to the wind -- we're exiting.
 				   This prevents asking the user dumb 
 				   questions.  */
   quit_command ((char *) 0, 0);
-  return 0;
 }
 #endif /* defined SIGHUP */
 
Index: top.h
===================================================================
RCS file: /cvs/src/src/gdb/top.h,v
retrieving revision 1.30
diff -u -p -r1.30 top.h
--- top.h	6 Sep 2011 14:48:59 -0000	1.30
+++ top.h	20 Sep 2011 15:29:37 -0000
@@ -41,7 +41,7 @@ extern void command_loop (void);
 extern int quit_confirm (void);
 extern void quit_force (char *, int);
 extern void quit_command (char *, int);
-extern int quit_cover (void *);
+extern void quit_cover (void);
 extern void execute_command (char *, int);
 
 /* Prepare for execution of a command.

-- 
Joseph S. Myers
joseph@codesourcery.com



More information about the Gdb-patches mailing list