On Mon, 11 Jul 2022 at 15:56, Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> wrote: > On 11/07/2022 14:39, Adam Dinwoodie wrote: > [snip] > > > > > Minor progress update: I've now confirmed the failure was introduced by > > a change in the Cygwin library between the binaries for Cygwin versions > > 3.2.0-1 and 3.3.1-1. Specifically, the test passes with Cygwin from the > > 27 October 2021 package archive[0], and fails with Cygwin from the 28 > > October 2021 archive[1], and the only difference between the two that > > has any chance of being relevant is that bump in the Cygwin release. > > Heh, I was just about to email you with similar news! I had a look at > my setup.log to see what I actually updated (and from what) and the > only thing that seemed to make sense was an update of the cygwin .dll > from v3.2.0-1 to v3.3.2-1 (I will add below an extract from my setup.log > for that day, in case you see anything else of interest). Having now spent far more time than I'd like wrangling the Cygwin build infrastructure, I've found the change in the Cygwin code that introduced the break. But I'm afraid progressing beyond this -- and in particular making any sort of judgement about appropriate next steps -- is beyond what I'm going to have time for in the foreseeable future. (On the plus side, this has given me the kick to actually work out how to do this sort of investigation, so if and when I get to investigating the other test failures that also seem to be caused by changes in the Cygwin environment, I now have a much better idea what I'm doing!) Relevant commit is below: https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=ef95c03522f65d5956a8dc82d869c6bc378ef3f9 commit ef95c03522f65d5956a8dc82d869c6bc378ef3f9 (HEAD, refs/bisect/bad) Author: Corinna Vinschen <corinna@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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,15 +1709,18 @@ 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); int ret = fh->evaluate_events (evt_mask, events, false); 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);
