public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
From: Corinna Vinschen <corinna@sourceware.org>
To: cygwin-cvs@sourceware.org
Subject: [newlib-cygwin] Cygwin: select: Fix FD_CLOSE handling
Date: Tue,  6 Apr 2021 19:36:03 +0000 (GMT)	[thread overview]
Message-ID: <20210406193603.B64203857823@sourceware.org> (raw)

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

commit ef95c03522f65d5956a8dc82d869c6bc378ef3f9
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Tue Apr 6 21:35:43 2021 +0200

    Cygwin: select: Fix FD_CLOSE handling
    
    An FD_CLOSE event sets a socket descriptor ready for writing.
    This is incorrect if the FD_CLOSE is a result of shutdown(SHUT_RD).
    Only set the socket descriptor ready for writing if the FD_CLOSE
    is indicating an connection abort or reset error condition.
    
    This requires to tweak fhandler_socket_wsock::evaluate_events.
    FD_CLOSE in conjunction with FD_ACCEPT/FD_CONNECT special cases
    a shutdown condition by setting an error code.  This is correct
    for accept/connect, but not for select.  In this case, make sure
    to return with an error code only if FD_CLOSE indicates a
    connection error.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/fhandler_socket_inet.cc | 26 ++++++++++++++++++--------
 winsup/cygwin/select.cc               |  7 +++++--
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/fhandler_socket_inet.cc b/winsup/cygwin/fhandler_socket_inet.cc
index bc08d3cf1..4ecb31a27 100644
--- a/winsup/cygwin/fhandler_socket_inet.cc
+++ b/winsup/cygwin/fhandler_socket_inet.cc
@@ -361,20 +361,30 @@ fhandler_socket_wsock::evaluate_events (const long event_mask, long &events,
 	  wsock_events->events |= FD_WRITE;
 	  wsock_events->connect_errorcode = 0;
 	}
-      /* 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)
 	{
-	  if ((event_mask & FD_ACCEPT) && saw_shutdown_read ())
+	  if (evts.iErrorCode[FD_CLOSE_BIT])
 	    {
-	      WSASetLastError (WSAEINVAL);
+	      WSASetLastError (evts.iErrorCode[FD_CLOSE_BIT]);
 	      ret = SOCKET_ERROR;
 	    }
-	  if (event_mask & FD_CONNECT)
+	  /* 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.  Note that
+	     we only do this when called from accept/connect, not from select.
+	     In this case erase == false, just as with read (MSG_PEEK). */
+	  if (erase)
 	    {
-	      WSASetLastError (WSAECONNRESET);
-	      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)
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 956cd9bc1..b493ccc11 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -1709,7 +1709,8 @@ peek_socket (select_record *me, bool)
   fhandler_socket_wsock *fh = (fhandler_socket_wsock *) me->fh;
   long events;
   /* Don't play with the settings again, unless having taken a deep look into
-     Richard W. Stevens Network Programming book.  Thank you. */
+     Richard W. Stevens Network Programming book and how these flags are
+     defined in Winsock.  Thank you. */
   long evt_mask = (me->read_selected ? (FD_READ | FD_ACCEPT | FD_CLOSE) : 0)
 		| (me->write_selected ? (FD_WRITE | FD_CONNECT | FD_CLOSE) : 0)
 		| (me->except_selected ? FD_OOB : 0);
@@ -1717,7 +1718,9 @@ peek_socket (select_record *me, bool)
   if (me->read_selected)
     me->read_ready |= ret || !!(events & (FD_READ | FD_ACCEPT | FD_CLOSE));
   if (me->write_selected)
-    me->write_ready |= ret || !!(events & (FD_WRITE | FD_CONNECT | FD_CLOSE));
+    /* Don't check for FD_CLOSE here.  Only an error case (ret == -1)
+       will set ready for writing. */
+    me->write_ready |= ret || !!(events & (FD_WRITE | FD_CONNECT));
   if (me->except_selected)
     me->except_ready |= !!(events & FD_OOB);


                 reply	other threads:[~2021-04-06 19:36 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210406193603.B64203857823@sourceware.org \
    --to=corinna@sourceware.org \
    --cc=cygwin-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).