From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31141 invoked by alias); 14 Oct 2008 22:03:49 -0000 Received: (qmail 31127 invoked by uid 22791); 14 Oct 2008 22:03:46 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 14 Oct 2008 22:03:11 +0000 Received: (qmail 10477 invoked from network); 14 Oct 2008 22:03:09 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 14 Oct 2008 22:03:09 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: PATCH: really close the extended-remote target if we lose the connection Date: Tue, 14 Oct 2008 22:03:00 -0000 User-Agent: KMail/1.9.9 MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_wcR9IiqaRkQiS+X" Message-Id: <200810142303.28790.pedro@codesourcery.com> 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: 2008-10/txt/msg00356.txt.bz2 --Boundary-00=_wcR9IiqaRkQiS+X Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 2069 Check out this difference between target remote, and target extended-remote: target remote: >./gdb /home/pedro/gdb/tests/threads32 GNU gdb (GDB) 6.8.50.20081014-cvs [...] (gdb) tar remote :9999 Remote debugging using :9999 0xf7fbb810 in ?? () from /lib/ld-linux.so.2 (gdb) info threads Remote connection closed (gdb) maint print target-stack The current target stack is: - exec (Local exec file) - None (None) (gdb) q [nothing] target extended-remote: >./gdb /home/pedro/gdb/tests/threads32 GNU gdb (GDB) 6.8.50.20081014-cvs [...] (gdb) tar extended-remote :9999 Remote debugging using :9999 0xf7f70810 in ?? () from /lib/ld-linux.so.2 (gdb) c Continuing. [Switching to Thread 22227] Remote connection closed (gdb) info threads putpkt: write failed: Broken pipe. (gdb) (gdb) maint print target-stack The current target stack is: - extended-remote (Extended remote serial target in gdb-specific protocol) - exec (Local exec file) - None (None) (gdb) (gdb) q The program is running. Quit anyway (and kill it)? (y or n) y Quitting: putpkt: write failed: Broken pipe. The issue is again the mixup of "target" as in 'interface'/'debug api'/'connection to system', vs "target" as in "inferior". In the remote target, a target_mourn_inferior unpushes the target_ops, while in the extended-remote target, it doesn't, leaving the user with a useless broken connection. The attached patch makes the extended-remote behave the same as the remote target. Considering an extended-remote connection debugging multi-processes seems to make it clearer that target_mourn_inferior isn't the right call here, me thinks. There are cases in async mode that when the connection was broken, we'd leave the SIGINT signal handler set to handle_remote_sigint or handle_remote_sigint_twice, although we had already poped the target, which would result in later crashes. I'm also making sure in remote_close that that doesn't happen. Any objections to this? No regressions on x86_64-unknown-linux-gnu (sync/async). -- Pedro Alves --Boundary-00=_wcR9IiqaRkQiS+X Content-Type: text/x-diff; charset="utf-8"; name="remote_close.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="remote_close.diff" Content-length: 3991 2008-10-14 Pedro Alves * remote.c (remote_close): Unregister remote_desc from the event loop. Always restore the SIGINT handler. Discard all inferiors here. (remote_detach_1, remote_disconnect): Don't unregister the file descriptor from the event loop here. (interrupt_query, readchar, getpkt_sane): Pop the target instead of morning the current inferior. (remote_kill): Don't unregister the file descriptor from the event loop here. (remote_mourn_1): Don't discard inferiors here. --- gdb/remote.c | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-) Index: src/gdb/remote.c =================================================================== --- src.orig/gdb/remote.c 2008-10-14 21:30:01.000000000 +0100 +++ src/gdb/remote.c 2008-10-14 22:55:52.000000000 +0100 @@ -2202,8 +2202,23 @@ static void remote_close (int quitting) { if (remote_desc) - serial_close (remote_desc); - remote_desc = NULL; + { + /* Unregister the file descriptor from the event loop. */ + if (target_is_async_p ()) + target_async (NULL, 0); + serial_close (remote_desc); + remote_desc = NULL; + } + + /* Make sure we don't leave the async SIGINT signal handler + installed. */ + signal (SIGINT, handle_sigint); + + /* We don't have a connection to the remote stub anymore. Get rid + of all the inferiors and their threads we were controlling. */ + discard_all_inferiors (); + + generic_mourn_inferior (); } /* Query the remote side for the text, data and bss offsets. */ @@ -3029,10 +3044,6 @@ remote_detach_1 (char *args, int from_tt else error (_("Can't detach process.")); - /* Unregister the file descriptor from the event loop. */ - if (target_is_async_p ()) - serial_async (remote_desc, NULL, 0); - if (from_tty) { if (remote_multi_process_p (rs)) @@ -3071,10 +3082,6 @@ remote_disconnect (struct target_ops *ta if (args) error (_("Argument given to \"disconnect\" when remotely debugging.")); - /* Unregister the file descriptor from the event loop. */ - if (target_is_async_p ()) - serial_async (remote_desc, NULL, 0); - /* Make sure we unpush even the extended remote targets; mourn won't do it. So call remote_mourn_1 directly instead of target_mourn_inferior. */ @@ -3546,8 +3553,7 @@ interrupt_query (void) if (query ("Interrupted while waiting for the program.\n\ Give up (and stop debugging it)? ")) { - target_mourn_inferior (); - signal (SIGINT, handle_sigint); + pop_target (); deprecated_throw_reason (RETURN_QUIT); } @@ -4954,7 +4960,7 @@ readchar (int timeout) switch ((enum serial_rc) ch) { case SERIAL_EOF: - target_mourn_inferior (); + pop_target (); error (_("Remote connection closed")); /* no return */ case SERIAL_ERROR: @@ -5387,7 +5393,7 @@ getpkt_sane (char **buf, long *sizeof_bu if (forever) /* Watchdog went off? Kill the target. */ { QUIT; - target_mourn_inferior (); + pop_target (); error (_("Watchdog timeout has expired. Target detached.")); } if (remote_debug) @@ -5437,10 +5443,6 @@ getpkt_sane (char **buf, long *sizeof_bu static void remote_kill (void) { - /* Unregister the file descriptor from the event loop. */ - if (target_is_async_p ()) - serial_async (remote_desc, NULL, 0); - /* Use catch_errors so the user can quit from gdb even when we aren't on speaking terms with the remote system. */ catch_errors ((catch_errors_ftype *) putpkt, "k", "", RETURN_MASK_ERROR); @@ -5512,12 +5514,9 @@ remote_mourn (void) static void remote_mourn_1 (struct target_ops *target) { - /* Get rid of all the inferiors and their threads we were - controlling. */ - discard_all_inferiors (); - unpush_target (target); - generic_mourn_inferior (); + + /* remote_close takes care of cleaning up. */ } static int --Boundary-00=_wcR9IiqaRkQiS+X--