DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org EEB883840C3D
Received-SPF: pass (mx2f80: connection is authenticated)
To: cygwin AT cygwin DOT com
Subject: [PATCH] Fix poll/select signal socket as write ready on connect
 failure
Message-ID: <efcd7b07-ccc3-a8a4-04c1-94f63012f042@marc-hoersken.de>
Date: Wed, 15 Jul 2020 20:54:40 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------71E77180C2D8C7988932DBF1"
From: Marc Hoersken via Cygwin <cygwin AT cygwin DOT com>
Reply-To: Marc Hoersken <info AT marc-hoersken DOT de>
Sender: "Cygwin" <cygwin-bounces AT cygwin DOT com>

This is a multi-part message in MIME format.
--------------71E77180C2D8C7988932DBF1
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: quoted-printable

Hello everyone,

I identified an issue related to the way the events FD_CONNECT and=20
FD_CLOSE returned by WSAEnumNetworkEvents are currently handled in=20
winsup/cygwin/fhandler_socket_inet.cc.

It seems like the code does not handle the fact that those events are=20
returned only once for a socket and if not acted upon by the calling=20
program may not be received again. This means poll and select are=20
currently not consistend about the socket still being writable after a=20
connect failure. The first call to poll or select would signal the=20
socket as writable, but not any following call. The first call consumes=20
the FD_CONNECT and FD_CLOSE events, regardless of the event mask=20
supplied by the calling program. So even if the calling program does not=20
care about writability in the first call, the events are consumed and=20
following calls checking for writability will not be able to detect a=20
connection failure.

A very simple test to reproduce can be made with the Cygwin provided=20
curl package. After installing with current Cygwin, issue the following=20
command to make it try connecting to a local non-listening port (eg. 47):

curl -v 127.0.0.1:47

With current Cygwin this will never timeout. An explanation of the curl=20
internals can be found here [1], but the short version is: curl waits on=20
sockets without checking/handling writability in a first poll call and=20
then after waiting, the writability (connection failure) is checked in a=20
second poll call per wait-loop iteration. Therefore curl can never=20
detect the connection failure in the second call, because the first call=20
already consumed the relevant events.

As far as I understand calling poll and/or select should not=20
change/reset the socket readyness state, therefore I created a simple=20
fix which could be used to solve this issue. Attached you will find a=20
suggested patch to make sure poll and select always signal writability=20
of a connection failed socket. With this patch applied the above example=20
command failed with a "Connection refused" as expected.

This patch only fixes the behaviour regarding connection failure (during=20
FD_CONNECT), I am not sure if connection closure (during FD_CLOSE) is=20
also affected, but I was not able to find code handling the fact that=20
FD_CLOSE is only signalled once.

Please take a look and thanks in advance!

Best regards,
Marc H=C3=B6rsken

[1] https://github.com/curl/curl/pull/5509#issuecomment-658357933


--------------71E77180C2D8C7988932DBF1
Content-Type: text/plain; charset=UTF-8;
 name="0001-Cygwin-make-sure-failed-sockets-always-signal-writab.patch"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename*0="0001-Cygwin-make-sure-failed-sockets-always-signal-writab.pa";
 filename*1="tch"

RnJvbSA3Y2Q5ZDU5N2EyYTMxNGMzYWViNWI3YzhhYWE5NzBkZWQ2ZDU2ZDdhIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJjIEhvZXJza2VuIDxpbmZvQG1hcmMtaG9lcnNr
ZW4uZGU+CkRhdGU6IFdlZCwgMTUgSnVsIDIwMjAgMjA6NTM6MjEgKzAyMDAKU3ViamVjdDog
W1BBVENIXSBDeWd3aW46IG1ha2Ugc3VyZSBmYWlsZWQgc29ja2V0cyBhbHdheXMgc2lnbmFs
IHdyaXRhYmlsaXR5CgpTaW5jZSBGRF9DT05ORUNUIGlzIG9ubHkgZ2l2ZW4gb25jZSwgd2Ug
bWFudWFsbHkgbmVlZCB0byBzZXQKRkRfV1JJVEUgZm9yIGNvbm5lY3Rpb24gZmFpbGVkIHNv
Y2tldHMgdG8gaGF2ZSBjb25zaXN0ZW50CmJlaGF2aW91ciBpbiBwcm9ncmFtcyBjYWxsaW5n
IHBvbGwvc2VsZWN0IG11bHRpcGxlIHRpbWVzLgoKRXhhbXBsZSB0ZXN0IHRvIG5vbi1saXN0
ZW5pbmcgcG9ydDogY3VybCAtdiAxMjcuMC4wLjE6NDcKLS0tCiB3aW5zdXAvY3lnd2luL2Zo
YW5kbGVyX3NvY2tldF9pbmV0LmNjIHwgNiArKysrKysKIDEgZmlsZSBjaGFuZ2VkLCA2IGlu
c2VydGlvbnMoKykKCmRpZmYgLS1naXQgYS93aW5zdXAvY3lnd2luL2ZoYW5kbGVyX3NvY2tl
dF9pbmV0LmNjIGIvd2luc3VwL2N5Z3dpbi9maGFuZGxlcl9zb2NrZXRfaW5ldC5jYwppbmRl
eCA3NGM0MTVkLi5lNWIwZDJkIDEwMDY0NAotLS0gYS93aW5zdXAvY3lnd2luL2ZoYW5kbGVy
X3NvY2tldF9pbmV0LmNjCisrKyBiL3dpbnN1cC9jeWd3aW4vZmhhbmRsZXJfc29ja2V0X2lu
ZXQuY2MKQEAgLTM3Niw2ICszNzYsMTIgQEAgZmhhbmRsZXJfc29ja2V0X3dzb2NrOjpldmFs
dWF0ZV9ldmVudHMgKGNvbnN0IGxvbmcgZXZlbnRfbWFzaywgbG9uZyAmZXZlbnRzLAogICAg
ICAgaWYgKGVyYXNlKQogCXdzb2NrX2V2ZW50cy0+ZXZlbnRzICY9IH4oZXZlbnRzICYgfihG
RF9XUklURSB8IEZEX0NMT1NFKSk7CiAgICAgfQorICAvKiBTaW5jZSBGRF9DT05ORUNUIGlz
IG9ubHkgZ2l2ZW4gb25jZSwgd2UgbWFudWFsbHkgbmVlZCB0byBzZXQKKyAgICAgRkRfV1JJ
VEUgZm9yIGNvbm5lY3Rpb24gZmFpbGVkIHNvY2tldHMgdG8gaGF2ZSBjb25zaXN0ZW50Cisg
ICAgIGJlaGF2aW91ciBpbiBwcm9ncmFtcyBjYWxsaW5nIHBvbGwvc2VsZWN0IG11bHRpcGxl
IHRpbWVzLgorICAgICBFeGFtcGxlIHRlc3QgdG8gbm9uLWxpc3RlbmluZyBwb3J0OiBjdXJs
IC12IDEyNy4wLjAuMTo0NyAqLworICBpZiAoKGNvbm5lY3Rfc3RhdGUgKCkgPT0gY29ubmVj
dF9mYWlsZWQpICYmIChldmVudF9tYXNrICYgRkRfV1JJVEUpKQorICAgIHdzb2NrX2V2ZW50
cy0+ZXZlbnRzIHw9IEZEX1dSSVRFOwogICBVTkxPQ0tfRVZFTlRTOwogCiAgIHJldHVybiBy
ZXQ7Ci0tIAoyLjcuNAoK
--------------71E77180C2D8C7988932DBF1
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

--
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

--------------71E77180C2D8C7988932DBF1--