This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC] Don't immediately SIGTERM the child of "target remote |".


Hi.

While testing a patch to allow gdbserver to communicate over stdio
(e.g. target remote | gdbserver ...)
I found that several tests from the testsuite were left running
and traced it to gdb SIGTERMing gdbserver when the connection is closed.

We could add a SIGTERM handler to gdbserver
but I'm starting with this patch.
I don't understand why gdb kills the child at all.
In a properly written child connection, closing stdin should be a
sufficient signal for the child to know to shutdown.

AFAICT The kill() call was added in cvs revision 1.2 with the log entry:
Replace ../include/wait.h with gdb_wait.h.
Blech.

Alas I can't remove this kill(SIGTERM) call
for fear of breaking something so I'm trying a
compromise and only killing the child after some timeout.

Comments?

I borrowed the SA_RESTART handling from other places in gdb.
I don't actually know that this works (i.e. the alarm call
will wake up waitpid) on systems without SA_RESTART,
and I don't know if alarm() is portable enough
(there are no current uses of it AFAICT).

2011-11-23  Doug Evans  <dje@google.com>

	* ser-pipe.c (sigalrm_handler): New function.
	(pipe_close): Don't immediately send SIGTERM to the child.
	Give it awhile to cleanly shutdown, but don't wait forever.

Index: ser-pipe.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-pipe.c,v
retrieving revision 1.32
diff -u -p -r1.32 ser-pipe.c
--- ser-pipe.c	4 Mar 2011 19:23:42 -0000	1.32
+++ ser-pipe.c	23 Nov 2011 23:46:55 -0000
@@ -154,6 +154,12 @@ pipe_open (struct serial *scb, const cha
 }
 
 static void
+sigalrm_handler (int signo)
+{
+  /* nothing to do */
+}
+
+static void
 pipe_close (struct serial *scb)
 {
   struct pipe_state *state = scb->state;
@@ -163,14 +169,45 @@ pipe_close (struct serial *scb)
 
   if (state != NULL)
     {
-      int status;
-      kill (state->pid, SIGTERM);
+      int rc, status;
+      void (*ofunc) ();	/* Previous SIGALRM handler.  */
+
+      /* Don't kill the task right away, give it a chance to shut down cleanly.
+	 But don't wait forever though.  */
+#if defined (HAVE_SIGACTION) && defined (SA_RESTART)
+      {
+	struct sigaction sa, osa;
+	sa.sa_handler = sigalrm_handler;
+	sigemptyset (&sa.sa_mask);
+	sa.sa_flags = 0;
+	sigaction (SIGALRM, &sa, &osa);
+	ofunc = osa.sa_handler;
+      }
+#else
+      ofunc = (void (*)()) signal (SIGALRM, sigalrm_handler);
+#endif
+
+#define PIPE_CLOSE_TIMEOUT 5
+      alarm (PIPE_CLOSE_TIMEOUT);
+
+      rc = -1;
 #ifdef HAVE_WAITPID
       /* Assume the program will exit after SIGTERM.  Might be
 	 useful to print any remaining stderr output from
 	 scb->error_fd while waiting.  */
-      waitpid (state->pid, &status, 0);
+      rc = waitpid (state->pid, &status, 0);
+#endif
+      if (rc != 0)
+	{
+	  kill (state->pid, SIGTERM);
+#ifdef HAVE_WAITPID
+	  waitpid (state->pid, &status, 0);
 #endif
+	}
+
+      alarm (0);
+      signal (SIGALRM, ofunc);
+
       if (scb->error_fd != -1)
 	close (scb->error_fd);
       scb->error_fd = -1;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]