X-Recipient: archive-cygwin AT delorie DOT com DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5525F388A837 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cygwin.com; s=default; t=1594891561; bh=TDjeWUWim1b2Tka45uGTU80uKCKUyFd32XQYyI3Kt9U=; h=Date:From:To:Subject:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: Reply-To:Cc:From; b=BRsJ7P17ljYaszBMDZcKcu2s3um9485ncF0qQxn2y22r7s2yZJm9PjJLFepvVICwV BwhHra786PIHfqHtCE/ch28/lMosVlTbk+PrDR920dfzCRPoyB1GB8fs2qQQFNmE0z SemAKX44P2UaivYaD915HMKbLojvC+TeBF0RTy48= X-Original-To: cygwin AT cygwin DOT com Delivered-To: cygwin AT cygwin DOT com DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9B532388A81D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=cygwin.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=corinna-cygwin AT cygwin DOT com Date: Thu, 16 Jul 2020 11:25:53 +0200 From: Corinna Vinschen To: Marc Hoersken Subject: Re: [PATCH] Fix poll/select signal socket as write ready on connect failure Message-ID: <20200716092553.GA3784@calimero.vinschen.de> Mail-Followup-To: Marc Hoersken , cygwin AT cygwin DOT com References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Provags-ID: V03:K1:SWUmSA8WtrlKljon4pZ9mbLd+0FEoG/UC6m2Jh8IwHhyvnOk30R 9OwHLf7vLAPNsEXvlRPdKxva88evYp1bERbyscBvhIo0PdgYXh3Sbo6etGe5VJg1JCieYSu wHBPcVDLrhdkYo3eBU8SsG1YSg5TWZKd2+9jLu5bICpOds4qjOkG4/Af+72E49e/BUR9PZU lgTjpYQQK9RnhSejJKEVA== X-UI-Out-Filterresults: notjunk:1;V03:K0:wnN6adIPFmM=:Ws38lScvBBWt5hkmsZD49u 2wgCfiEz0k4Vi6GT3ubPRzxmbCoWY/SbSBtrAtMF2MrDnVZtHNWNCKVFOCWLJs1isnF1xFicQ +bFv7stOZcxlIP/Zt37yEXfHjd6lP/O4akI5eRimAO30LosrPbR+5jFw6smokcPBh30aXOVgb 57NK/RklsdvDEkSx6Z86CGiJQw9a1LcSwK1xGj+aA6IzACzRZNAAPQJX06FwzfKJ0H65+1fYf fqcsddaOaoT+PiFWYUeRf8qvZ0aSIKwYRuWP/O1vFdTz+1NnKXhaGnmZRlqtrxGMBazQbonHd eIidNNYoMYzaB7B+oJUEnRuf+NHsrEUQ+kh250C4oybdt/XNqqExJqjHG71Tajv7g2ZfqxWCq Ou8UYzMV9+eZ/dyvzEtr5qW8HBVa9CiQWN6ANv4TGq4ssrMdxhlDTFRNreu0VyXFgCZ1xzc90 gbyyxrDM7505jT6vaK6a1TEn5ElLPQD6F8QB7lCOuyuLxWnvfVg7up7tJ4vTnSMhJAKcpZIsd CJ3mOPzSHxx19JbzxJffhYPoQp+Z2pP2IJkzviG3iBLvbPGzK6PZGMvW3ZKw5lE///Eu67kn2 lt6LEY12YS+7L1jOHlIXfOsiExTlO6/a1JTwkhIu1+REayCtZD7+IRg3tprMqt236Sgj2JR3G L3+tg3VFgAN342gWtqpXBeABOmrSBgMk56DYX0tGxZWYNr0REkzoZffzGyEPwz875pkPmqBe2 KayVrUV61rQ4oqO272LCH6D2uIVR5eDuS86JQnu9+A3dFLWGYySYKBMQwbJq1EnRYvO75wVAR iA0ckEiyUE2PDrgYwcZj/AtjGZKiPd2H6oorMBPIYZO7whl3/9Mb0kFrd4GSM2HmCab3NPWJd PqYFaDPfymLKbPjc8jIw== X-Spam-Status: No, score=-103.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GOOD_FROM_CORINNA_CYGWIN, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: cygwin AT cygwin DOT com X-Mailman-Version: 2.1.29 Precedence: list List-Id: General Cygwin discussions and problem reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: cygwin AT cygwin DOT com Cc: cygwin AT cygwin DOT com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: cygwin-bounces AT cygwin DOT com Sender: "Cygwin" Hi Marc, On Jul 15 20:54, Marc Hoersken via Cygwin wrote: > Hello everyone, > > I identified an issue related to the way the events FD_CONNECT and FD_CLOSE > returned by WSAEnumNetworkEvents are currently handled in > winsup/cygwin/fhandler_socket_inet.cc. > > It seems like the code does not handle the fact that those events are > returned only once for a socket and if not acted upon by the calling program > may not be received again. This means poll and select are currently not > consistend about the socket still being writable after a connect failure. > The first call to poll or select would signal the socket as writable, but > not any following call. The first call consumes the FD_CONNECT and FD_CLOSE > events, regardless of the event mask supplied by the calling program. So > even if the calling program does not care about writability in the first > call, the events are consumed and following calls checking for writability > will not be able to detect a connection failure. > [...] > As far as I understand calling poll and/or select should not change/reset > the socket readyness state, therefore I created a simple fix which could be > used to solve this issue. Attached you will find a suggested patch to make > sure poll and select always signal writability of a connection failed > socket. With this patch applied the above example command failed with a > "Connection refused" as expected. > > This patch only fixes the behaviour regarding connection failure (during > FD_CONNECT), I am not sure if connection closure (during FD_CLOSE) is also > affected, but I was not able to find code handling the fact that FD_CLOSE is > only signalled once. > > Please take a look and thanks in advance! Thanks for the patch. I pushed it. But then I got second thoughts in terms of how to fix the issue. The reason is that the FD_CLOSE problem shouldn't exist, simply for the fact that we never remove FD_CLOSE from the events mask, see https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/fhandler_socket_inet.cc;hb=HEAD#l377 So, rather than setting FD_WRITE at some later point in the code, what about handling this where the other FD_CONNECT stuff is handled, by not erasing the FD_CONNECT bit, just like with FD_CLOSE? diff --git a/winsup/cygwin/fhandler_socket_inet.cc b/winsup/cygwin/fhandler_socket_inet.cc index e5b0d2d1443e..b64d96225db1 100644 --- a/winsup/cygwin/fhandler_socket_inet.cc +++ b/winsup/cygwin/fhandler_socket_inet.cc @@ -354,7 +354,12 @@ fhandler_socket_wsock::evaluate_events (const long event_mask, long &events, } else wsock_events->events |= FD_WRITE; - wsock_events->events &= ~FD_CONNECT; + /* Since FD_CONNECT is only given once, we have to keep FD_CONNECT + for connection failed sockets to have consistent behaviour in + programs calling poll/select multiple times. Example test to + non-listening port: curl -v 127.0.0.1:47 */ + if (connect_state () != connect_failed) + wsock_events->events &= ~FD_CONNECT; wsock_events->connect_errorcode = 0; } /* This test makes accept/connect behave as on Linux when accept/connect @@ -376,12 +381,6 @@ fhandler_socket_wsock::evaluate_events (const long event_mask, long &events, if (erase) wsock_events->events &= ~(events & ~(FD_WRITE | FD_CLOSE)); } - /* Since FD_CONNECT is only given once, we manually need to set - FD_WRITE for connection failed sockets to have consistent - behaviour in programs calling poll/select multiple times. - Example test to non-listening port: curl -v 127.0.0.1:47 */ - if ((connect_state () == connect_failed) && (event_mask & FD_WRITE)) - wsock_events->events |= FD_WRITE; UNLOCK_EVENTS; return ret; What do you think? Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer -- Problem reports: https://cygwin.com/problems.html FAQ: https://cygwin.com/faq/ Documentation: https://cygwin.com/docs.html Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple