This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
PATCH: really close the extended-remote target if we lose the connection
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Tue, 14 Oct 2008 23:03:28 +0100
- Subject: PATCH: really close the extended-remote target if we lose the connection
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
2008-10-14 Pedro Alves <pedro@codesourcery.com>
* 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