From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14780 invoked by alias); 24 Nov 2011 00:01:14 -0000 Received: (qmail 14770 invoked by uid 22791); 24 Nov 2011 00:01:13 -0000 X-SWARE-Spam-Status: No, hits=-3.2 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-bw0-f73.google.com (HELO mail-bw0-f73.google.com) (209.85.214.73) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 24 Nov 2011 00:00:55 +0000 Received: by bkbzs8 with SMTP id zs8so71863bkb.0 for ; Wed, 23 Nov 2011 16:00:54 -0800 (PST) Received: by 10.14.10.85 with SMTP id 61mr1742274eeu.4.1322092854512; Wed, 23 Nov 2011 16:00:54 -0800 (PST) Received: by 10.14.10.85 with SMTP id 61mr1742270eeu.4.1322092854398; Wed, 23 Nov 2011 16:00:54 -0800 (PST) Received: from hpza10.eem.corp.google.com ([74.125.121.33]) by gmr-mx.google.com with ESMTPS id q50si7344104eef.3.2011.11.23.16.00.54 (version=TLSv1/SSLv3 cipher=AES128-SHA); Wed, 23 Nov 2011 16:00:54 -0800 (PST) Received: from wpaz5.hot.corp.google.com (wpaz5.hot.corp.google.com [172.24.198.69]) by hpza10.eem.corp.google.com (Postfix) with ESMTPS id 144F8200055 for ; Wed, 23 Nov 2011 16:00:53 -0800 (PST) Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.18.110.50]) by wpaz5.hot.corp.google.com with ESMTP id pAO00qqb032334 for ; Wed, 23 Nov 2011 16:00:52 -0800 Received: by ruffy.mtv.corp.google.com (Postfix, from userid 67641) id 14DC12461B1; Wed, 23 Nov 2011 16:00:51 -0800 (PST) To: gdb-patches@sourceware.org Subject: [RFC] Don't immediately SIGTERM the child of "target remote |". Message-Id: <20111124000052.14DC12461B1@ruffy.mtv.corp.google.com> Date: Thu, 24 Nov 2011 00:01:00 -0000 From: dje@google.com (Doug Evans) X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-11/txt/msg00665.txt.bz2 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 * 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;