* cygserver_transport_pipes.cc: The main change is to make the

	client try harder to connect to the server if it's previously
	connected, and so has good grounds for believing that the server
	is running.
	(MAX_WAIT_NAMED_PIPE_RETRY): Change to be an enumerator.
	(WAIT_NAMED_PIPE_TIMEOUT): Ditto.
	(transport_layer_pipes::accept): Use interlocked operators on
	`pipe_instance'.
	(transport_layer_pipes::close): Rearrange so that FlushFileBuffers
	and DisconnectNamedPipe are only called for accepted endpoints.
	Use interlocked operators on `pipe_instance'.
	(transport_layer_pipes::read): Use set_errno where required.
	(transport_layer_pipes::write): Ditto.
	(transport_layer_pipes::connect): Add local static variable
	`assume_cygserver'.  Set it if a connection is made to cygserver,
	clear it if a connection is not made even after retrying.  If set,
	ignore all errors from CreateFile and retry the connection.  Catch
	the situation where WaitNamedPipe fails to wait [sic] and add a
	`Sleep (0)' so that the server gets a chance to run.

2 files changed, 76 insertions, 27 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 57d5cbcda..58bf53e0e 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,25 @@
+2002-07-09  Conrad Scott  <conrad.scott@dsl.pipex.com>
+
+	* cygserver_transport_pipes.cc: The main change is to make the
+	client try harder to connect to the server if it's previously
+	connected, and so has good grounds for believing that the server
+	is running.
+	(MAX_WAIT_NAMED_PIPE_RETRY): Change to be an enumerator.
+	(WAIT_NAMED_PIPE_TIMEOUT): Ditto.
+	(transport_layer_pipes::accept): Use interlocked operators on
+	`pipe_instance'.
+	(transport_layer_pipes::close): Rearrange so that FlushFileBuffers
+	and DisconnectNamedPipe are only called for accepted endpoints.
+	Use interlocked operators on `pipe_instance'.
+	(transport_layer_pipes::read): Use set_errno where required.
+	(transport_layer_pipes::write): Ditto.
+	(transport_layer_pipes::connect): Add local static variable
+	`assume_cygserver'.  Set it if a connection is made to cygserver,
+	clear it if a connection is not made even after retrying.  If set,
+	ignore all errors from CreateFile and retry the connection.  Catch
+	the situation where WaitNamedPipe fails to wait [sic] and add a
+	`Sleep (0)' so that the server gets a chance to run.
+
 2002-07-09  Christopher Faylor  <cgf@redhat.com>
 
 	* debug.cc: Avoid explicit zeroing of globals.
diff --git a/winsup/cygwin/cygserver_transport_pipes.cc b/winsup/cygwin/cygserver_transport_pipes.cc
index 54728b91e..2ea0cdbd5 100755
--- a/winsup/cygwin/cygserver_transport_pipes.cc
+++ b/winsup/cygwin/cygserver_transport_pipes.cc
@@ -25,6 +25,7 @@ details. */
 #include <pthread.h>
 #include <unistd.h>
 
+#include "cygerrno.h"
 #include "cygwin/cygserver_transport.h"
 #include "cygwin/cygserver_transport_pipes.h"
 
@@ -32,8 +33,11 @@ details. */
 #include "cygwin/cygserver.h"
 #endif
 
-static const int MAX_WAIT_NAMED_PIPE_RETRY = 64;
-static const DWORD WAIT_NAMED_PIPE_TIMEOUT = 100; // milliseconds
+enum
+{
+  MAX_WAIT_NAMED_PIPE_RETRY = 64,
+  WAIT_NAMED_PIPE_TIMEOUT = 10	// milliseconds
+};
 
 #ifndef __INSIDE_CYGWIN__
 
@@ -134,7 +138,7 @@ transport_layer_pipes::accept (bool *const recoverable)
 			  && GetLastError () == ERROR_ACCESS_DENIED);
 
   if (accept_pipe != INVALID_HANDLE_VALUE)
-    pipe_instance += 1;
+    InterlockedIncrement (&pipe_instance);
 
   LeaveCriticalSection (&pipe_instance_lock);
 
@@ -173,29 +177,30 @@ void
 transport_layer_pipes::close()
 {
   // verbose: debug_printf ("closing pipe %p", pipe);
+
   if (pipe)
     {
       assert (pipe != INVALID_HANDLE_VALUE);
 
-      FlushFileBuffers (pipe);
-      DisconnectNamedPipe (pipe);
-#ifndef __INSIDE_CYGWIN__
       if (is_accepted_endpoint)
 	{
+#ifndef __INSIDE_CYGWIN__
+	  (void) FlushFileBuffers (pipe); // Blocks until client reads.
+	  (void) DisconnectNamedPipe (pipe);
 	  EnterCriticalSection (&pipe_instance_lock);
 	  (void) CloseHandle (pipe);
 	  assert (pipe_instance > 0);
-	  pipe_instance -= 1;
+	  InterlockedDecrement (&pipe_instance);
 	  LeaveCriticalSection (&pipe_instance_lock);
+#else /* __INSIDE_CYGWIN__ */
+	  assert (false);
+#endif /* __INSIDE_CYGWIN__ */
 	}
       else
 	(void) CloseHandle (pipe);
-#else
-      (void) CloseHandle (pipe);
-#endif
-    }
 
-  pipe = NULL;
+      pipe = NULL;
+    }
 }
 
 ssize_t
@@ -205,7 +210,7 @@ transport_layer_pipes::read (void * const buf, const size_t len)
 
   if (!pipe)
     {
-      errno = EBADF;
+      set_errno (EBADF);
       return -1;
     }
 
@@ -215,7 +220,7 @@ transport_layer_pipes::read (void * const buf, const size_t len)
   if (!ReadFile (pipe, buf, len, &count, NULL))
     {
       debug_printf ("error reading from pipe (%lu)", GetLastError ());
-      errno = EINVAL;		// FIXME?
+      set_errno (EINVAL);	// FIXME?
       return -1;
     }
 
@@ -229,7 +234,7 @@ transport_layer_pipes::write (void * const buf, const size_t len)
 
   if (!pipe)
     {
-      errno = EBADF;
+      set_errno (EBADF);
       return -1;
     }
 
@@ -238,14 +243,23 @@ transport_layer_pipes::write (void * const buf, const size_t len)
   DWORD count;
   if (!WriteFile (pipe, buf, len, &count, NULL))
     {
-      debug_printf ("error writing to pipe (%lu)", GetLastError ());
-      errno = EINVAL;		// FIXME?
+      debug_printf ("error writing to pipe, error = %lu", GetLastError ());
+      set_errno (EINVAL);	// FIXME?
       return -1;
     }
 
   return count;
 }
 
+/*
+ * This routine holds a static variable, assume_cygserver, that is set
+ * if the transport has good reason to think that cygserver is
+ * running, i.e. if if successfully connected to it with the previous
+ * attempt.  If this is set, the code tries a lot harder to get a
+ * connection, making the assumption that any failures are just
+ * congestion and overloading problems.
+ */
+
 bool
 transport_layer_pipes::connect ()
 {
@@ -257,46 +271,59 @@ transport_layer_pipes::connect ()
       return false;
     }
 
+  static bool assume_cygserver = false;
+
   BOOL rc = TRUE;
   int retries = 0;
 
   while (rc)
     {
       pipe = CreateFile (pipe_name,
-			 GENERIC_READ | GENERIC_WRITE,
-			 FILE_SHARE_READ | FILE_SHARE_WRITE,
-			 &sec_all_nih,
-			 OPEN_EXISTING,
-			 SECURITY_IMPERSONATION,
-			 NULL);
+			   GENERIC_READ | GENERIC_WRITE,
+			   FILE_SHARE_READ | FILE_SHARE_WRITE,
+			   &sec_all_nih,
+			   OPEN_EXISTING,
+			   SECURITY_IMPERSONATION,
+			   NULL);
 
       if (pipe != INVALID_HANDLE_VALUE)
 	{
 	  assert (pipe);
-	  /* got the pipe */
+	  assume_cygserver = true;
 	  return true;
 	}
 
-      if (GetLastError () != ERROR_PIPE_BUSY)
+      if (!assume_cygserver && GetLastError () != ERROR_PIPE_BUSY)
 	{
 	  debug_printf ("Error opening the pipe (%lu)", GetLastError ());
 	  pipe = NULL;
 	  return false;
 	}
 
-      (void) CloseHandle (pipe);
       pipe = NULL;
 
+      /* Note: `If no instances of the specified named pipe exist, the
+       * WaitNamedPipe function returns immediately, regardless of the
+       * time-out value.'  Thus the explicit Sleep if the call fails
+       * with ERROR_FILE_NOT_FOUND.
+       */
       while (retries != MAX_WAIT_NAMED_PIPE_RETRY
 	     && !(rc = WaitNamedPipe (pipe_name, WAIT_NAMED_PIPE_TIMEOUT)))
 	{
+	  if (GetLastError () == ERROR_FILE_NOT_FOUND)
+	    Sleep (0);		// Give the server a chance.
+
 	  retries += 1;
 	}
     }
 
-  system_printf ("Pipe busy and retry limit reached, error = %lu",
+  assert (retries == MAX_WAIT_NAMED_PIPE_RETRY);
+
+  system_printf ("lost connection to cygserver, error = %lu",
 		 GetLastError ());
 
+  assume_cygserver = false;
+
   return false;
 }