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 <corinna-cygwin AT cygwin DOT com>
To:	Marc Hoersken <info AT marc-hoersken DOT de>
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 <info AT marc-hoersken DOT de>, cygwin AT cygwin DOT com
References:	<efcd7b07-ccc3-a8a4-04c1-94f63012f042 AT marc-hoersken DOT de>
MIME-Version:	1.0
In-Reply-To:	<efcd7b07-ccc3-a8a4-04c1-94f63012f042@marc-hoersken.de>
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
List-Id:	General Cygwin discussions and problem reports <cygwin.cygwin.com>
List-Unsubscribe:	<http://cygwin.com/mailman/options/cygwin>,
	<mailto:cygwin-request AT cygwin DOT com?subject=unsubscribe>
List-Archive:	<https://cygwin.com/pipermail/cygwin/>
List-Post:	<mailto:cygwin AT cygwin DOT com>
List-Help:	<mailto:cygwin-request AT cygwin DOT com?subject=help>
List-Subscribe:	<http://cygwin.com/mailman/listinfo/cygwin>,
	<mailto:cygwin-request AT cygwin DOT com?subject=subscribe>
Reply-To:	cygwin AT cygwin DOT com
Cc:	cygwin AT cygwin DOT com
Errors-To:	cygwin-bounces AT cygwin DOT com
Sender:	"Cygwin" <cygwin-bounces AT cygwin DOT com>

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

- Raw text -