[newlib-cygwin] Try to handle concurrent close on socket more gracefully

[Corinna Vinschen <corinna at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=79d65a1ed2eccd7c9c280895b8dc99ff7112b3c0

commit 79d65a1ed2eccd7c9c280895b8dc99ff7112b3c0
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Mon Jun 15 18:29:56 2015 +0200

    Try to handle concurrent close on socket more gracefully
    
    	* fhandler_socket.cc (LOCK_EVENTS): Don't enter critical section with
    	invalid mutex handle since then socket has been closed.
    	(UNLOCK_EVENTS): Close critical section.
    	(fhandler_socket::evaluate_events): Handle calling connect on shutdown
    	socket.
    	(fhandler_socket::wait_for_events): Try for pthread_testcancel in case
    	of WAIT_FAILED.  Try to come up with a better errno in case we waited
    	on an invalid handle.
    	(fhandler_socket::release_events): Change wsock_mtx and wsock_evt to
    	NULL under lock to avoid accessing invalid handle.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/ChangeLog          | 13 ++++++++++
 winsup/cygwin/fhandler_socket.cc | 54 +++++++++++++++++++++++++++++-----------
 2 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index bb6da15..062b065 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,18 @@
 2015-06-15  Corinna Vinschen  <corinna@vinschen.de>
 
+	* fhandler_socket.cc (LOCK_EVENTS): Don't enter critical section with
+	invalid mutex handle since then socket has been closed.
+	(UNLOCK_EVENTS): Close critical section.
+	(fhandler_socket::evaluate_events): Handle calling connect on shutdown
+	socket.
+	(fhandler_socket::wait_for_events): Try for pthread_testcancel in case
+	of WAIT_FAILED.  Try to come up with a better errno in case we waited
+	on an invalid handle.
+	(fhandler_socket::release_events): Change wsock_mtx and wsock_evt to
+	NULL under lock to avoid accessing invalid handle.
+
+2015-06-15  Corinna Vinschen  <corinna@vinschen.de>
+
 	* net.cc (errmap): Handle more Winsock error codes.
 
 2015-06-15  Corinna Vinschen  <corinna@vinschen.de>
diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc
index 1b28e52..48f9aeb 100644
--- a/winsup/cygwin/fhandler_socket.cc
+++ b/winsup/cygwin/fhandler_socket.cc
@@ -501,8 +501,14 @@ fhandler_socket::af_local_set_secret (char *buf)
    counted as one socket. */
 #define NUM_SOCKS       (32768 / sizeof (wsa_event))
 
-#define LOCK_EVENTS	WaitForSingleObject (wsock_mtx, INFINITE)
-#define UNLOCK_EVENTS	ReleaseMutex (wsock_mtx)
+#define LOCK_EVENTS	\
+  if (wsock_mtx && \
+      WaitForSingleObject (wsock_mtx, INFINITE) != WAIT_FAILED) \
+    {
+
+#define UNLOCK_EVENTS \
+      ReleaseMutex (wsock_mtx); \
+    }
 
 static wsa_event wsa_events[NUM_SOCKS] __attribute__((section (".cygwin_dll_common"), shared));
 
@@ -709,14 +715,21 @@ fhandler_socket::evaluate_events (const long event_mask, long &events,
 	  wsock_events->events &= ~FD_CONNECT;
 	  wsock_events->connect_errorcode = 0;
 	}
-      /* This test makes the accept function behave as on Linux when
-	 accept is called on a socket for which shutdown for the read side
-	 has been called.  The second half of this code is in the shutdown
-	 method.  See there for more info. */
-      if ((event_mask & FD_ACCEPT) && (events & FD_CLOSE))
+      /* This test makes accept/connect behave as on Linux when accept/connect
+         is called on a socket for which shutdown has been called.  The second
+	 half of this code is in the shutdown method. */
+      if (events & FD_CLOSE)
 	{
-	  WSASetLastError (WSAEINVAL);
-	  ret = SOCKET_ERROR;
+	  if ((event_mask & FD_ACCEPT) && saw_shutdown_read ())
+	    {
+	      WSASetLastError (WSAEINVAL);
+	      ret = SOCKET_ERROR;
+	    }
+	  if (event_mask & FD_CONNECT)
+	    {
+	      WSASetLastError (WSAECONNRESET);
+	      ret = SOCKET_ERROR;
+	    }
 	}
       if (erase)
 	wsock_events->events &= ~(events & ~(FD_WRITE | FD_CLOSE));
@@ -750,7 +763,8 @@ fhandler_socket::wait_for_events (const long event_mask, const DWORD flags)
 	{
 	  case WSA_WAIT_TIMEOUT:
 	    pthread_testcancel ();
-	    /*FALLTHRU*/
+	    break;
+
 	  case WSA_WAIT_EVENT_0:
 	    break;
 
@@ -761,19 +775,31 @@ fhandler_socket::wait_for_events (const long event_mask, const DWORD flags)
 	    return SOCKET_ERROR;
 
 	  default:
-	    WSASetLastError (WSAEFAULT);
+	    pthread_testcancel ();
+	    /* wsock_evt can be NULL.  We're generating the same errno values
+	       as for sockets on which shutdown has been called. */
+	    if (WSAGetLastError () != WSA_INVALID_HANDLE)
+	      WSASetLastError (WSAEFAULT);
+	    else
+	      WSASetLastError ((event_mask & FD_CONNECT) ? WSAECONNRESET
+							 : WSAEINVAL);
 	    return SOCKET_ERROR;
 	}
     }
-
   return ret;
 }
 
 void
 fhandler_socket::release_events ()
 {
-  NtClose (wsock_evt);
-  NtClose (wsock_mtx);
+  HANDLE evt = wsock_evt;
+  HANDLE mtx = wsock_mtx;
+
+  LOCK_EVENTS;
+  wsock_evt = wsock_mtx = NULL;
+  } ReleaseMutex (mtx);	/* == UNLOCK_EVENTS, but note using local mtx here. */
+  NtClose (evt);
+  NtClose (mtx);
 }
 
 /* Called from net.cc:fdsock() if a freshly created socket is not