Re: RFA: auto-retry TCP connections, take 2

[Sandra Loosemore <sandra at codesourcery dot com>]

Sandra Loosemore wrote:
> Here's a new patch, revised per the previous discussion to rename the 
> new commands.  Moving them out of "set/show remote" into a new "set/show 
> tcp" prefix  cleaned up the implementation logic a bit, so it's probably 
> the right choice.
>
> The rest of the patch, with the actual retry/timeout logic, is unchanged 
> from the previous version.
>
> OK to check in?

It's been brought to my attention that the patch I sent previously was in an 
inconvenient format.  Here's a better one, I hope.

-Sandra

2008-12-30  Sandra Loosemore  <sandra@codesourcery.com>

	gdb/
	* ser-tcp.c: Adjust includes.
	(tcp_set_cmdlist, tcp_show_cmdlist): Declare.
	(tcp_auto_retry, tcp_retry_limit): Declare.
	(TIMEOUT): Remove, in favor of tcp_retry_limit.
	(POLL_INTERVAL): Increase to 5, in favor of backoff logic.
	(wait_for_connect): New function.
	(net_open): Use it.  Add auto-retry logic.
	(set_tcp_cmd, show_tcp_cmd): New functions.
	(_initialize_ser_tcp): Initialize new "set/show tcp auto-retry"
	and "set/show tcp connect-timeout" commands.
	* NEWS: Document new commands.

	gdb/doc/
	* gdb.texinfo (Remote Configuration): Document new
	"set/show tcp auto-retry" and "set/show tcp connect-timeout"
	commands.

Index: gdb/NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.296
diff -u -r1.296 NEWS
--- gdb/NEWS	2 Dec 2008 07:57:36 -0000	1.296
+++ gdb/NEWS	31 Dec 2008 14:19:31 -0000
@@ -169,6 +169,14 @@
   with GDB while the target is running.  "show target-async" displays the
   current state of asynchronous execution of the target.
 
+set tcp auto-retry (on|off)
+show tcp auto-retry
+set tcp connect-timeout
+show tcp connect-timeout
+  These commands allow GDB to retry failed TCP connections to a remote stub
+  with a specified timeout period; this is useful if the stub is launched
+  in parallel with GDB but may not be ready to accept connections immediately.
+
 macro define
 macro list
 macro undef
Index: gdb/ser-tcp.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-tcp.c,v
retrieving revision 1.30
diff -u -r1.30 ser-tcp.c
--- gdb/ser-tcp.c	3 Sep 2008 23:54:19 -0000	1.30
+++ gdb/ser-tcp.c	31 Dec 2008 14:19:31 -0000
@@ -22,6 +22,9 @@
 #include "serial.h"
 #include "ser-base.h"
 #include "ser-tcp.h"
+#include "gdbcmd.h"
+#include "cli/cli-decode.h"
+#include "cli/cli-setshow.h"
 
 #include <sys/types.h>
 
@@ -49,6 +52,7 @@
 
 #include <signal.h>
 #include "gdb_string.h"
+#include "gdb_select.h"
 
 #ifndef HAVE_SOCKLEN_T
 typedef int socklen_t;
@@ -56,10 +60,93 @@
 
 void _initialize_ser_tcp (void);
 
-/* seconds to wait for connect */
-#define TIMEOUT 15
+/* For "set tcp" and "show tcp".  */
+
+static struct cmd_list_element *tcp_set_cmdlist;
+static struct cmd_list_element *tcp_show_cmdlist;
+
+/* Whether to auto-retry refused connections.  */
+
+static int tcp_auto_retry = 1;
+
+/* Timeout period for connections, in seconds.  */
+
+static int tcp_retry_limit = 15;
+
 /* how many times per second to poll deprecated_ui_loop_hook */
-#define POLL_INTERVAL 2
+
+#define POLL_INTERVAL 5
+
+/* Helper function to wait a while.  If SCB is non-null, wait on its
+   file descriptor.  Otherwise just wait on a timeout, updating *POLLS.
+   Returns -1 on timeout or interrupt, otherwise the value of select.  */
+
+static int
+wait_for_connect (struct serial *scb, int *polls)
+{
+  struct timeval t;
+  int n;
+
+  /* While we wait for the connect to complete, 
+     poll the UI so it can update or the user can 
+     interrupt.  */
+  if (deprecated_ui_loop_hook && deprecated_ui_loop_hook (0))
+    {
+      errno = EINTR;
+      return -1;
+    }
+
+  /* Check for timeout.  */
+  if (*polls > tcp_retry_limit * POLL_INTERVAL)
+    {
+      errno = ETIMEDOUT;
+      return -1;
+    }
+
+  /* Back off to polling once per second after the first POLL_INTERVAL
+     polls.  */
+  if (*polls < POLL_INTERVAL)
+    {
+      t.tv_sec = 0;
+      t.tv_usec = 1000000 / POLL_INTERVAL;
+    }
+  else
+    {
+      t.tv_sec = 1;
+      t.tv_usec = 0;
+    }
+
+  if (scb)
+    {
+      fd_set rset, wset, eset;
+      FD_ZERO (&rset);
+      FD_SET (scb->fd, &rset);
+      wset = rset;
+      eset = rset;
+	  
+      /* POSIX systems return connection success or failure by signalling
+	 wset.  Windows systems return success in wset and failure in
+	 eset.
+     
+	 We must call select here, rather than gdb_select, because
+	 the serial structure has not yet been initialized - the
+	 MinGW select wrapper will not know that this FD refers
+	 to a socket.  */
+      n = select (scb->fd + 1, &rset, &wset, &eset, &t);
+    }
+  else
+    /* Use gdb_select here, since we have no file descriptors, and on
+       Windows, plain select doesn't work in that case.  */
+    n = gdb_select (0, NULL, NULL, NULL, &t);
+
+  /* If we didn't time out, only count it as one poll.  */
+  if (n > 0 || *polls < POLL_INTERVAL)
+    (*polls)++;
+  else
+    (*polls) += POLL_INTERVAL;
+
+  return n;
+}
 
 /* Open a tcp socket */
 
@@ -76,6 +163,7 @@
 #else
   int ioarg;
 #endif
+  int polls = 0;
 
   use_udp = 0;
   if (strncmp (name, "udp:", 4) == 0)
@@ -108,6 +196,13 @@
       return -1;
     }
 
+  sockaddr.sin_family = PF_INET;
+  sockaddr.sin_port = htons (port);
+  memcpy (&sockaddr.sin_addr.s_addr, hostent->h_addr,
+	  sizeof (struct in_addr));
+
+ retry:
+
   if (use_udp)
     scb->fd = socket (PF_INET, SOCK_DGRAM, 0);
   else
@@ -116,11 +211,6 @@
   if (scb->fd < 0)
     return -1;
   
-  sockaddr.sin_family = PF_INET;
-  sockaddr.sin_port = htons (port);
-  memcpy (&sockaddr.sin_addr.s_addr, hostent->h_addr,
-	  sizeof (struct in_addr));
-
   /* set socket nonblocking */
   ioarg = 1;
   ioctl (scb->fd, FIONBIO, &ioarg);
@@ -128,68 +218,51 @@
   /* Use Non-blocking connect.  connect() will return 0 if connected already. */
   n = connect (scb->fd, (struct sockaddr *) &sockaddr, sizeof (sockaddr));
 
-  if (n < 0
+  if (n < 0)
+    {
 #ifdef USE_WIN32API
-      /* Under Windows, calling "connect" with a non-blocking socket
-	 results in WSAEWOULDBLOCK, not WSAEINPROGRESS.  */
-      && WSAGetLastError() != WSAEWOULDBLOCK
+      int err = WSAGetLastError();
 #else
-      && errno != EINPROGRESS
+      int err = errno;
 #endif
-      )
-    {
+
+      /* Maybe we're waiting for the remote target to become ready to
+	 accept connections.  */
+      if (tcp_auto_retry
 #ifdef USE_WIN32API
-      errno = WSAGetLastError();
+	  && err == WSAECONNREFUSED
+#else
+	  && err == ECONNREFUSED
 #endif
-      net_close (scb);
-      return -1;
-    }
+	  && wait_for_connect (NULL, &polls) >= 0)
+	{
+	  close (scb->fd);
+	  goto retry;
+	}
 
-  if (n)
-    {
-      /* looks like we need to wait for the connect */
-      struct timeval t;
-      fd_set rset, wset, eset;
-      int polls = 0;
-      FD_ZERO (&rset);
+      if (
+#ifdef USE_WIN32API
+	  /* Under Windows, calling "connect" with a non-blocking socket
+	     results in WSAEWOULDBLOCK, not WSAEINPROGRESS.  */
+	  err != WSAEWOULDBLOCK
+#else
+	  err != EINPROGRESS
+#endif
+	  )
+	{
+	  errno = err;
+	  net_close (scb);
+	  return -1;
+	}
 
+      /* looks like we need to wait for the connect */
       do 
 	{
-	  /* While we wait for the connect to complete, 
-	     poll the UI so it can update or the user can 
-	     interrupt.  */
-	  if (deprecated_ui_loop_hook)
-	    {
-	      if (deprecated_ui_loop_hook (0))
-		{
-		  errno = EINTR;
-		  net_close (scb);
-		  return -1;
-		}
-	    }
-	  
-	  FD_SET (scb->fd, &rset);
-	  wset = rset;
-	  eset = rset;
-	  t.tv_sec = 0;
-	  t.tv_usec = 1000000 / POLL_INTERVAL;
-	  
-	  /* POSIX systems return connection success or failure by signalling
-	     wset.  Windows systems return success in wset and failure in
-	     eset.
-
-	     We must call select here, rather than gdb_select, because
-	     the serial structure has not yet been initialized - the
-	     MinGW select wrapper will not know that this FD refers
-	     to a socket.  */
-	  n = select (scb->fd + 1, &rset, &wset, &eset, &t);
-	  polls++;
+	  n = wait_for_connect (scb, &polls);
 	} 
-      while (n == 0 && polls <= TIMEOUT * POLL_INTERVAL);
-      if (n < 0 || polls > TIMEOUT * POLL_INTERVAL)
+      while (n == 0);
+      if (n < 0)
 	{
-	  if (polls > TIMEOUT * POLL_INTERVAL)
-	    errno = ETIMEDOUT;
 	  net_close (scb);
 	  return -1;
 	}
@@ -207,6 +280,18 @@
     res = getsockopt (scb->fd, SOL_SOCKET, SO_ERROR, (void *) &err, &len);
     if (res < 0 || err)
       {
+	/* Maybe the target still isn't ready to accept the connection.  */
+	if (tcp_auto_retry
+#ifdef USE_WIN32API
+	    && err == WSAECONNREFUSED
+#else
+	    && err == ECONNREFUSED
+#endif
+	    && wait_for_connect (NULL, &polls) >= 0)
+	  {
+	    close (scb->fd);
+	    goto retry;
+	  }
 	if (err)
 	  errno = err;
 	net_close (scb);
@@ -264,13 +349,27 @@
   return (serial_write (scb, "\377\363", 2));
 }
 
+/* Support for "set tcp" and "show tcp" commands.  */
+
+static void
+set_tcp_cmd (char *args, int from_tty)
+{
+  help_list (tcp_set_cmdlist, "set tcp ", -1, gdb_stdout);
+}
+
+static void
+show_tcp_cmd (char *args, int from_tty)
+{
+  help_list (tcp_show_cmdlist, "show tcp ", -1, gdb_stdout);
+}
+
+
 void
 _initialize_ser_tcp (void)
 {
 #ifdef USE_WIN32API
   /* Do nothing; the TCP serial operations will be initialized in
      ser-mingw.c.  */
-  return;
 #else
   struct serial_ops *ops;
   ops = XMALLOC (struct serial_ops);
@@ -297,4 +396,29 @@
   ops->write_prim = net_write_prim;
   serial_add_interface (ops);
 #endif /* USE_WIN32API */
+
+  add_prefix_cmd ("tcp", class_maintenance, set_tcp_cmd, _("\
+TCP protocol specific variables\n\
+Configure variables specific to remote TCP connections"),
+		  &tcp_set_cmdlist, "set tcp ",
+		  0 /* allow-unknown */, &setlist);
+  add_prefix_cmd ("tcp", class_maintenance, show_tcp_cmd, _("\
+TCP protocol specific variables\n\
+Configure variables specific to remote TCP connections"),
+		  &tcp_show_cmdlist, "show tcp ",
+		  0 /* allow-unknown */, &showlist);
+
+  add_setshow_boolean_cmd ("auto-retry", class_obscure,
+			   &tcp_auto_retry, _("\
+Set auto-retry on socket connect"), _("\
+Show auto-retry on socket connect"), 
+			   NULL, NULL, NULL,
+			   &tcp_set_cmdlist, &tcp_show_cmdlist);
+
+  add_setshow_uinteger_cmd ("connect-timeout", class_obscure,
+			    &tcp_retry_limit, _("\
+Set timeout limit for socket connection"), _("\
+Show timeout limit for socket connection"),
+			   NULL, NULL, NULL,
+			   &tcp_set_cmdlist, &tcp_show_cmdlist);
 }
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.541
diff -u -r1.541 gdb.texinfo
--- gdb/doc/gdb.texinfo	16 Dec 2008 06:14:00 -0000	1.541
+++ gdb/doc/gdb.texinfo	31 Dec 2008 14:19:32 -0000
@@ -14206,6 +14206,36 @@
 extended-remote}.  This should be set to a filename valid on the
 target system.  If it is not set, the target will use a default
 filename (e.g.@: the last program run).
+
+@kindex set tcp
+@kindex show tcp
+@item set tcp auto-retry on
+@cindex auto-retry, for remote TCP target
+Enable auto-retry for remote TCP connections.  This is useful if the remote
+debugging agent is launched in parallel with @value{GDBN}; there is a race
+condition because the agent may not become ready to accept the connection
+before @value{GDBN} attempts to connect.  When auto-retry is
+enabled, if the initial attempt to connect fails, @value{GDBN} reattempts
+to establish the connection using the timeout specified by 
+@code{set tcp connect-timeout}.
+
+@item set tcp auto-retry off
+Do not auto-retry failed TCP connections.
+
+@item show tcp auto-retry
+Show the current auto-retry setting.
+
+@item set tcp connect-timeout @var{seconds}
+@cindex connection timeout, for remote TCP target
+@cindex timeout, for remote target connection
+Set the timeout for establishing a TCP connection to the remote target to
+@var{seconds}.  The timeout affects both polling to retry failed connections 
+(enabled by @code{set tcp auto-retry on}) and waiting for connections
+that are merely slow to complete, and represents an approximate cumulative
+value.
+
+@item show tcp connect-timeout
+Show the current connection timeout setting.
 @end table
 
 @cindex remote packets, enabling and disabling