Re: [patch] [gdbserver] Fix multi-GB error log files [doc review]

Jan Kratochvil
2011-04-23T08:00:59Z

On Sat, 23 Apr 2011 08:15:38 +0200, Eli Zaretskii wrote:
> GDBserver now keeps its listening port open, unless you start
> GDBserver with the --once option. Keeping the listening port open
> allows subsequent reconnections from another GDB session.

BTW personally I read it as if GDBserver did not support reconnections before
and that reconnections are a new feature (which they are not).

> > + Option @option{--multi} is not directly related
> > +to @kbd{target extended-remote} (therefore @kbd{target remote} is also
> > +compatible) and vice versa.
>
> It's not clear to me how this sentence is related to the surrounding
> text. What potential problem did you try to prevent by that?

Yes, I was considering to post it as a separate patch but (a) it would be
a hassle for one sentence and (b) it is all related together anyway.

--multi, remote/extended-remote and --once all together affect when gdbserver
will / will not terminate. Normally AFAIK one uses either:
--multi with extended-remote
or
(default) with remote
so I was curious if one can mix the two modes. And to my surprise gdbserver
behavior in which cases it terminates depends on how GDB connects to it
(remote/extended-remote) instead of the gdbserver arguments (--multi).
This is why this question seemed to me logical in the context of explaining
all the GDB termination rules which needed to be explained together with the
port allocation rules.

> > +terminated in @kbd{target remote} mode. On the other hand for @kbd{target
> ^
> Missing comma there.

You wrote it as "On the other, hand for " but I have used "On the other hand,
for ", either a typo or a font layout problem on your side.

> > + The purpose of option
> > +@option{--once} is to enable reusing the same port number for connections to
> > +multiple @code{gdbserver}s running at the same host.
>
> "The purpose of the @option{--once} option is ..."
>
> However, is the above the only purpose of this option? If not, saying
> "the purpose" is inaccurate.

Not the only one, it is also useful if any unexpected situation happens in the
testsuite gdbserver will no longer be needlessly running.

> How about the following rewording:

Used.

> * gdb.texinfo (Server): Improve indexing. Index all optional
> switches to gdbserver.

Added @cindex for --once.

Used all your other suggestions, I am not aware much of the commas.

I will check it in if no futher comments appear.

Thanks,
Jan

gdb/gdbserver/
2011-04-23 Jan Kratochvil <jan.kratochvil@redhat.com>

* NEWS: Document the new gdbserver --once option.
* remote-utils.c (handle_accept_event): Close LISTEN_DESC only if
RUN_ONCE. Comment for the LISTEN_DESC delete_file_handler call.
(remote_prepare): New function with most of the TCP code from ...
(remote_open): ... here. Detect PORT here unconditionally. Move also
setting transport_is_reliable.
* server.c (run_once): New variable.
(gdbserver_usage): Document it.
(main): Set run_once for `--once'. Call remote_prepare. Exit after
the first run if RUN_ONCE.
* server.h (run_once, remote_prepare): New declarations.

gdb/doc/
2011-04-23 Jan Kratochvil <jan.kratochvil@redhat.com>
Eli Zaretskii <eliz@gnu.org>

* gdb.texinfo (Starting and Stopping Trace Experiments): New anchor
for disconnected tracing.
(Multi-Process Mode for @code{gdbserver}): Mention --multi and
extended-remote relationship. Mention --once.
(TCP port allocation lifecycle of @code{gdbserver}): New.

gdb/testsuite/
2011-04-17 Jan Kratochvil <jan.kratochvil@redhat.com>

* gdb.base/solib-disc.exp: Set gdbserver_reconnect_p.
* lib/gdb.exp (gdb_init): Clear gdbserver_reconnect_p.
* lib/gdbserver-support.exp (gdbserver_start): Add `--once' if
!gdbserver_reconnect_p..
(gdbserver_reconnect): Call error if !gdbserver_reconnect_p..

--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,10 @@

*** Changes since GDB 7.3

+* GDBserver now keeps its listening port open, unless you start GDBserver with
+ the --once option. Keeping the listening port open allows subsequent
+ reconnections from another GDB session.
+
*** Changes in GDB 7.3

* GDB has a new command: "thread find [REGEXP]".
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10435,6 +10435,7 @@ Enter actions for tracepoint #1, one per line.
(@value{GDBP}) @b{tstop}
@end smallexample

+@anchor{disconnected tracing}
@cindex disconnected tracing
You can choose to continue running the trace experiment even if
@value{GDBN} disconnects from the target, voluntarily or
@@ -16230,11 +16231,43 @@ redirection (@pxref{Arguments}).
To start @code{gdbserver} without supplying an initial command to run
or process ID to attach, use the @option{--multi} command line option.
Then you can connect using @kbd{target extended-remote} and start
-the program you want to debug.
-
-@code{gdbserver} does not automatically exit in multi-process mode.
-You can terminate it by using @code{monitor exit}
-(@pxref{Monitor Commands for gdbserver}).
+the program you want to debug. The @option{--multi} option is not directly
+related to @kbd{target extended-remote} (therefore @kbd{target remote} is also
+compatible) and vice versa.
+
+In multi-process mode @code{gdbserver} does not automatically exit unless you
+use the option @option{--once}. You can terminate it by using
+@code{monitor exit} (@pxref{Monitor Commands for gdbserver}).
+
+@subsubsection TCP port allocation lifecycle of @code{gdbserver}
+
+This section applies only when @code{gdbserver} is run to listen on a TCP port.
+
+@code{gdbserver} normally terminates after all of its debugged processes have
+terminated in @kbd{target remote} mode. On the other hand, for @kbd{target
+extended-remote}, @code{gdbserver} stays running even with no processes left.
+@value{GDBN} normally terminates the spawned debugged process on its exit,
+which normally also terminates @code{gdbserver} in the @kbd{target remote}
+mode. Therefore, when the connection drops unexpectedly, and @value{GDBN}
+cannot ask @code{gdbserver} to kill its debugged processes, @code{gdbserver}
+stays running even in the @kbd{target remote} mode.
+
+When @code{gdbserver} stays running, @value{GDBN} can connect to it again later.
+Such reconnecting is useful for features like @ref{disconnected tracing}. For
+completeness, at most one @value{GDBN} can be connected at a time.
+
+@cindex @option{--once}, @code{gdbserver} option
+By default, @code{gdbserver} keeps the listening TCP port open, so that
+additional connections are possible. However, if you start @code{gdbserver}
+with the @option{--once} option, it will stop listening for any further
+connection attempts after connecting to the first @value{GDBN} session. This
+means no further connections to @code{gdbserver} will be possible after the
+first one. It also means @code{gdbserver} will terminate after the first
+connection with remote @value{GDBN} has closed, even for unexpectedly closed
+connections and even in the @kbd{target extended-remote} mode. The
+@option{--once} option allows reusing the same port number for connecting to
+multiple instances of @code{gdbserver} running on the same host, since each
+instance closes its port after the first connection.

@subsubsection Other Command-Line Arguments for @code{gdbserver}

--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -170,14 +170,21 @@ handle_accept_event (int err, gdb_client_data client_data)
(char *) &tmp, sizeof (tmp));

#ifndef USE_WIN32API
- close (listen_desc); /* No longer need this */
-
signal (SIGPIPE, SIG_IGN); /* If we don't do this, then gdbserver simply
exits when the remote side dies. */
+#endif
+
+ if (run_once)
+ {
+#ifndef USE_WIN32API
+ close (listen_desc); /* No longer need this */
#else
- closesocket (listen_desc); /* No longer need this */
+ closesocket (listen_desc); /* No longer need this */
#endif
+ }

+ /* Even if !RUN_ONCE no longer notice new connections. Still keep the
+ descriptor open for add_file_handler to wait for a new connection. */
delete_file_handler (listen_desc);

/* Convert IP address to string. */
@@ -200,6 +207,62 @@ handle_accept_event (int err, gdb_client_data client_data)
return 0;
}

+/* Prepare for a later connection to a remote debugger.
+ NAME is the filename used for communication. */
+
+void
+remote_prepare (char *name)
+{
+ char *port_str;
+#ifdef USE_WIN32API
+ static int winsock_initialized;
+#endif
+ int port;
+ struct sockaddr_in sockaddr;
+ socklen_t tmp;
+ char *port_end;
+
+ port_str = strchr (name, ':');
+ if (port_str == NULL)
+ {
+ transport_is_reliable = 0;
+ return;
+ }
+
+ port = strtoul (port_str + 1, &port_end, 10);
+ if (port_str[1] == '\0' || *port_end != '\0')
+ fatal ("Bad port argument: %s", name);
+
+#ifdef USE_WIN32API
+ if (!winsock_initialized)
+ {
+ WSADATA wsad;
+
+ WSAStartup (MAKEWORD (1, 0), &wsad);
+ winsock_initialized = 1;
+ }
+#endif
+
+ listen_desc = socket (PF_INET, SOCK_STREAM, IPPROTO_TCP);
+ if (listen_desc == -1)
+ perror_with_name ("Can't open socket");
+
+ /* Allow rapid reuse of this port. */
+ tmp = 1;
+ setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp,
+ sizeof (tmp));
+
+ sockaddr.sin_family = PF_INET;
+ sockaddr.sin_port = htons (port);
+ sockaddr.sin_addr.s_addr = INADDR_ANY;
+
+ if (bind (listen_desc, (struct sockaddr *) &sockaddr, sizeof (sockaddr))
+ || listen (listen_desc, 1))
+ perror_with_name ("Can't bind address");
+
+ transport_is_reliable = 1;
+}
+
/* Open a connection to a remote debugger.
NAME is the filename used for communication. */

@@ -274,8 +337,6 @@ remote_open (char *name)

fprintf (stderr, "Remote debugging using %s\n", name);

- transport_is_reliable = 0;
-
enable_async_notification (remote_desc);

/* Register the event loop handler. */
@@ -284,64 +345,22 @@ remote_open (char *name)
}
else
{
-#ifdef USE_WIN32API
- static int winsock_initialized;
-#endif
int port;
+ socklen_t len;
struct sockaddr_in sockaddr;
- socklen_t tmp;
- char *port_end;

- port = strtoul (port_str + 1, &port_end, 10);
- if (port_str[1] == '\0' || *port_end != '\0')
- fatal ("Bad port argument: %s", name);
-
-#ifdef USE_WIN32API
- if (!winsock_initialized)
- {
- WSADATA wsad;
-
- WSAStartup (MAKEWORD (1, 0), &wsad);
- winsock_initialized = 1;
- }
-#endif
-
- listen_desc = socket (PF_INET, SOCK_STREAM, IPPROTO_TCP);
- if (listen_desc == -1)
- perror_with_name ("Can't open socket");
-
- /* Allow rapid reuse of this port. */
- tmp = 1;
- setsockopt (listen_desc, SOL_SOCKET, SO_REUSEADDR, (char *) &tmp,
- sizeof (tmp));
-
- sockaddr.sin_family = PF_INET;
- sockaddr.sin_port = htons (port);
- sockaddr.sin_addr.s_addr = INADDR_ANY;
-
- if (bind (listen_desc, (struct sockaddr *) &sockaddr, sizeof (sockaddr))
- || listen (listen_desc, 1))
- perror_with_name ("Can't bind address");
-
- /* If port is zero, a random port will be selected, and the
- fprintf below needs to know what port was selected. */
- if (port == 0)
- {
- socklen_t len = sizeof (sockaddr);
- if (getsockname (listen_desc,
- (struct sockaddr *) &sockaddr, &len) < 0
- || len < sizeof (sockaddr))
- perror_with_name ("Can't determine port");
- port = ntohs (sockaddr.sin_port);
- }
+ len = sizeof (sockaddr);
+ if (getsockname (listen_desc,
+ (struct sockaddr *) &sockaddr, &len) < 0
+ || len < sizeof (sockaddr))
+ perror_with_name ("Can't determine port");
+ port = ntohs (sockaddr.sin_port);

fprintf (stderr, "Listening on port %d\n", port);
fflush (stderr);

/* Register the event loop handler. */
add_file_handler (listen_desc, handle_accept_event, NULL);
-
- transport_is_reliable = 1;
}
}

--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -40,6 +40,9 @@ static int extended_protocol;
static int response_needed;
static int exit_requested;

+/* --once: Exit after the first connection has closed. */
+int run_once;
+
int multi_process;
int non_stop;

@@ -2312,7 +2315,9 @@ gdbserver_usage (FILE *stream)
" --debug Enable general debugging output.\n"
" --remote-debug Enable remote protocol debugging output.\n"
" --version Display version information and exit.\n"
- " --wrapper WRAPPER -- Run WRAPPER to start new programs.\n");
+ " --wrapper WRAPPER -- Run WRAPPER to start new programs.\n"
+ " --once Exit after the first connection has "
+ "closed.\n");
if (REPORT_BUGS_TO[0] && stream == stdout)
fprintf (stream, "Report bugs to \"%s\".\n", REPORT_BUGS_TO);
}
@@ -2536,6 +2541,8 @@ main (int argc, char *argv[])
}
}
}
+ else if (strcmp (*next_arg, "--once") == 0)
+ run_once = 1;
else
{
fprintf (stderr, "Unknown argument: %s\n", *next_arg);
@@ -2648,6 +2655,8 @@ main (int argc, char *argv[])
exit (1);
}

+ remote_prepare (port);
+
while (1)
{
noack_mode = 0;
@@ -2676,7 +2685,7 @@ main (int argc, char *argv[])
getpkt to fail; close the connection and reopen it at the
top of the loop. */

- if (exit_requested)
+ if (exit_requested || run_once)
{
detach_or_kill_for_exit ();
exit (0);
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -355,6 +355,7 @@ extern int disable_packet_Tthread;
extern int disable_packet_qC;
extern int disable_packet_qfThreadInfo;

+extern int run_once;
extern int multi_process;
extern int non_stop;

@@ -406,6 +407,7 @@ int putpkt (char *buf);
int putpkt_binary (char *buf, int len);
int putpkt_notif (char *buf);
int getpkt (char *buf);
+void remote_prepare (char *name);
void remote_open (char *name);
void remote_close (void);
void write_ok (char *buf);
--- a/gdb/testsuite/gdb.base/solib-disc.exp
+++ b/gdb/testsuite/gdb.base/solib-disc.exp
@@ -19,6 +19,7 @@ if {[skip_shlib_tests]} {
return 0
}

+set gdbserver_reconnect_p 1
if { [info proc gdb_reconnect] == "" } {
return 0
}
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2809,6 +2809,11 @@ proc gdb_init { args } {
# especially having color output turned on can cause tests to fail.
setenv GREP_OPTIONS ""

+ # Clear $gdbserver_reconnect_p.
+ global gdbserver_reconnect_p
+ set gdbserver_reconnect_p 1
+ unset gdbserver_reconnect_p
+
return [eval default_gdb_init $args];
}

--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -218,10 +218,21 @@ proc gdbserver_start { options arguments } {

# Fire off the debug agent.
set gdbserver_command "$gdbserver"
+
+ # If gdbserver_reconnect will be called $gdbserver_reconnect_p must be
+ # set to true already during gdbserver_start.
+ global gdbserver_reconnect_p
+ if {![info exists gdbserver_reconnect_p] || !$gdbserver_reconnect_p} {
+ # GDB client could accidentally connect to a stale server.
+ append gdbserver_command " --once"
+ }
+
if { $options != "" } {
append gdbserver_command " $options"
}
+
append gdbserver_command " :$portnum"
+
if { $arguments != "" } {
append gdbserver_command " $arguments"
}
@@ -315,6 +326,12 @@ proc gdbserver_reconnect { } {
global gdbserver_protocol
global gdbserver_gdbport

+ global gdbserver_reconnect_p;
+ if {![info exists gdbserver_reconnect_p] || !$gdbserver_reconnect_p} {
+ error "gdbserver_reconnect_p is not set before gdbserver_reconnect"
+ return 0
+ }
+
return [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
}