scp stalls on uploading in cygwin 3.5 current master.

scp stalls on uploading in cygwin 3.5 current master.

[Takashi Yano]

Hi Corinna,

After the commit dedbbd74d0a8, "scp file server:." stalls.
I confirmed this when the "server" is a Linux machine.
The problem does not occur if it is reverted.

Could you please have a look?

commit dedbbd74d0a8f3b7dfae6188321703a47bb8a2b3
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Tue Aug 1 14:22:55 2023 +0200

    Cygwin: select: workaround FD_WRITE network event handling
    
    The FD_WRITE event is a false friend.  It indicates ready to write
    even if the next send fails with WSAEWOULDBLOCK.  *After* the fact,
    FD_WRITE will be cleared until sending is again possible, but that
    is too late for a select/write loop.
    
    Workaround that by using the WinSock select function when peeking
    at a socket and FD_WRITE gets indicated. WinSock select fortunately
    indicates writability correctly.
    
    Fixes: 70e476d27be8 ("(peek_socket): Use event handling for peeking socket.")
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>


-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: scp stalls on uploading in cygwin 3.5 current master.

[Takashi Yano]

On Thu, 24 Aug 2023 06:05:02 +0900
Takashi Yano wrote:
> Hi Corinna,
> 
> After the commit dedbbd74d0a8, "scp file server:." stalls.
> I confirmed this when the "server" is a Linux machine.
> The problem does not occur if it is reverted.
> 
> Could you please have a look?
> 
> commit dedbbd74d0a8f3b7dfae6188321703a47bb8a2b3
> Author: Corinna Vinschen <corinna@vinschen.de>
> Date:   Tue Aug 1 14:22:55 2023 +0200
> 
>     Cygwin: select: workaround FD_WRITE network event handling
>     
>     The FD_WRITE event is a false friend.  It indicates ready to write
>     even if the next send fails with WSAEWOULDBLOCK.  *After* the fact,
>     FD_WRITE will be cleared until sending is again possible, but that
>     is too late for a select/write loop.
>     
>     Workaround that by using the WinSock select function when peeking
>     at a socket and FD_WRITE gets indicated. WinSock select fortunately
>     indicates writability correctly.
>     
>     Fixes: 70e476d27be8 ("(peek_socket): Use event handling for peeking socket.")
>     Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

I'm not sure why at all, however, the following patch seems to
solve the issue.

diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 7b9473849..de5794c9f 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -1790,7 +1790,7 @@ peek_socket (select_record *me, bool)
       if (events & FD_WRITE)
 	{
 	  wfd_set w = { 1, { fh->get_socket () } };
-	  TIMEVAL t = { 0 };
+	  TIMEVAL t = { .tv_sec = 0, .tv_usec = 1 };
 
 	  if (_win32_select (0, NULL, &w, NULL, &t) == 0)
 	    events &= ~FD_WRITE;

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: scp stalls on uploading in cygwin 3.5 current master.

[Corinna Vinschen]

On Aug 24 12:31, Takashi Yano via Cygwin wrote:
> On Thu, 24 Aug 2023 06:05:02 +0900
> Takashi Yano wrote:
> > Hi Corinna,
> > 
> > After the commit dedbbd74d0a8, "scp file server:." stalls.
> > I confirmed this when the "server" is a Linux machine.
> > The problem does not occur if it is reverted.
> > 
> > Could you please have a look?
> > 
> > commit dedbbd74d0a8f3b7dfae6188321703a47bb8a2b3
> > Author: Corinna Vinschen <corinna@vinschen.de>
> > Date:   Tue Aug 1 14:22:55 2023 +0200
> > 
> >     Cygwin: select: workaround FD_WRITE network event handling
> >     
> >     The FD_WRITE event is a false friend.  It indicates ready to write
> >     even if the next send fails with WSAEWOULDBLOCK.  *After* the fact,
> >     FD_WRITE will be cleared until sending is again possible, but that
> >     is too late for a select/write loop.
> >     
> >     Workaround that by using the WinSock select function when peeking
> >     at a socket and FD_WRITE gets indicated. WinSock select fortunately
> >     indicates writability correctly.
> >     
> >     Fixes: 70e476d27be8 ("(peek_socket): Use event handling for peeking socket.")
> >     Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
> 
> I'm not sure why at all, however, the following patch seems to
> solve the issue.
> 
> diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
> index 7b9473849..de5794c9f 100644
> --- a/winsup/cygwin/select.cc
> +++ b/winsup/cygwin/select.cc
> @@ -1790,7 +1790,7 @@ peek_socket (select_record *me, bool)
>        if (events & FD_WRITE)
>  	{
>  	  wfd_set w = { 1, { fh->get_socket () } };
> -	  TIMEVAL t = { 0 };
> +	  TIMEVAL t = { .tv_sec = 0, .tv_usec = 1 };
>  
>  	  if (_win32_select (0, NULL, &w, NULL, &t) == 0)
>  	    events &= ~FD_WRITE;

Yeah, this is weird. A TIMEVAL value of 0 indicates non-blocking,
so why should waiting a usec make that better?  It also potentially
slows down Cygwin's select noticably if multiple sockets are part
of the descriptor set.

Hmmm.

Is it possible that _win32_select returns with SOCKET_ERROR for 
some reason?

Unfortunately I'm a bit swamped ATM, but rather than setting t to 1
usec, what if the check goes:

  if (_win32_select (0, NULL, &w, NULL, &t) != 1)

?


Thanks,
Corinna

Re: scp stalls on uploading in cygwin 3.5 current master.

[Takashi Yano]

On Thu, 24 Aug 2023 10:59:33 +0200
Corinna Vinschen wrote:
> > I'm not sure why at all, however, the following patch seems to
> > solve the issue.
> > 
> > diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
> > index 7b9473849..de5794c9f 100644
> > --- a/winsup/cygwin/select.cc
> > +++ b/winsup/cygwin/select.cc
> > @@ -1790,7 +1790,7 @@ peek_socket (select_record *me, bool)
> >        if (events & FD_WRITE)
> >  	{
> >  	  wfd_set w = { 1, { fh->get_socket () } };
> > -	  TIMEVAL t = { 0 };
> > +	  TIMEVAL t = { .tv_sec = 0, .tv_usec = 1 };
> >  
> >  	  if (_win32_select (0, NULL, &w, NULL, &t) == 0)
> >  	    events &= ~FD_WRITE;
> 
> Yeah, this is weird. A TIMEVAL value of 0 indicates non-blocking,
> so why should waiting a usec make that better?  It also potentially
> slows down Cygwin's select noticably if multiple sockets are part
> of the descriptor set.
> 
> Hmmm.
> 
> Is it possible that _win32_select returns with SOCKET_ERROR for 
> some reason?
> 
> Unfortunately I'm a bit swamped ATM, but rather than setting t to 1
> usec, what if the check goes:
> 
>   if (_win32_select (0, NULL, &w, NULL, &t) != 1)
> 
> ?

This did not help. I looked into this deeper and noticed that:
1) _win32_select() sometimes returns 0.
2) If _win32_select() returns 0, WaitForMultipleObjects(..., INFINITE)
   is called in thread_socket().
3) WaitForMultipleObjects() sometimes does not return for FD_WRITE
   for unknown reason.
This causes the stall.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: scp stalls on uploading in cygwin 3.5 current master.

[Corinna Vinschen]

On Aug 25 17:48, Takashi Yano via Cygwin wrote:
> On Thu, 24 Aug 2023 10:59:33 +0200
> Corinna Vinschen wrote:
> > > I'm not sure why at all, however, the following patch seems to
> > > solve the issue.
> > > 
> > > diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
> > > index 7b9473849..de5794c9f 100644
> > > --- a/winsup/cygwin/select.cc
> > > +++ b/winsup/cygwin/select.cc
> > > @@ -1790,7 +1790,7 @@ peek_socket (select_record *me, bool)
> > >        if (events & FD_WRITE)
> > >  	{
> > >  	  wfd_set w = { 1, { fh->get_socket () } };
> > > -	  TIMEVAL t = { 0 };
> > > +	  TIMEVAL t = { .tv_sec = 0, .tv_usec = 1 };
> > >  
> > >  	  if (_win32_select (0, NULL, &w, NULL, &t) == 0)
> > >  	    events &= ~FD_WRITE;
> > 
> > Yeah, this is weird. A TIMEVAL value of 0 indicates non-blocking,
> > so why should waiting a usec make that better?  It also potentially
> > slows down Cygwin's select noticably if multiple sockets are part
> > of the descriptor set.
> > 
> > Hmmm.
> > 
> > Is it possible that _win32_select returns with SOCKET_ERROR for 
> > some reason?
> > 
> > Unfortunately I'm a bit swamped ATM, but rather than setting t to 1
> > usec, what if the check goes:
> > 
> >   if (_win32_select (0, NULL, &w, NULL, &t) != 1)
> > 
> > ?
> 
> This did not help. I looked into this deeper and noticed that:
> 1) _win32_select() sometimes returns 0.
> 2) If _win32_select() returns 0, WaitForMultipleObjects(..., INFINITE)
>    is called in thread_socket().
> 3) WaitForMultipleObjects() sometimes does not return for FD_WRITE
>    for unknown reason.
> This causes the stall.

So the situation is that the network event handling returned FD_WRITE,
because it always returns FD_WRITE as long as a non-blocking send()
function didn't explicitely fail due to buffer overrun.

However, _win32_select will notice that the buffer is full, so it
does not return 1, but 0.  I e., the socket is not ready for writing.

Now you're saying that it's possible that the following WFMO will
never return?

That would mean that the FD_WRITE event won't be triggered again because
it already *had* been triggered and the only way to re-enable it is to
call one of the send() functions (see
https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-wsaeventselect)

I don't have an answer to this problem yet.

Can we use send(sock, "", 0) to reenable FD_WRITE, perhaps?


Corinna

RE: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Lavrentiev, Anton (NIH/NLM/NCBI) [C]]

> I don't have an answer to this problem yet.
> 
> Can we use send(sock, "", 0) to reenable FD_WRITE, perhaps?

Can't it just be assumed that the socket is _always_ writeable _unless_ the last send() failed?
In other words, try to always send() if it did not fail before.  If it did, only send() after
FD_WRITE was returned in the event mask.

We do that in our code and it works without any issues with any sorts of timeouts (including none).

Anton Lavrentiev
Contractor NIH/NLM/NCBI

Re: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Corinna Vinschen]

On Aug 25 12:08, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin wrote:
> > I don't have an answer to this problem yet.
> > 
> > Can we use send(sock, "", 0) to reenable FD_WRITE, perhaps?
> 
> Can't it just be assumed that the socket is _always_ writeable _unless_ the last send() failed?
> In other words, try to always send() if it did not fail before.  If it did, only send() after
> FD_WRITE was returned in the event mask.

You're looking from the application perspective, but as the underlying
library we don't have the application under control.  The application
can rightfully expect POSIX-like behaviour from select(2), and *that*
means, it can expect select(2) to return a socket as non-writable if the
internal buffer is full, *before* it calls send:

  while (...)
    {
      /* send as long as we can, otherwise do another job in the meantime */
      while (select (..., <zero timeout>))
	send (<blocking>);
      <do something else>
    }


Corinna

Re: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Corinna Vinschen]

On Aug 25 14:23, Corinna Vinschen via Cygwin wrote:
> On Aug 25 12:08, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin wrote:
> > > I don't have an answer to this problem yet.
> > > 
> > > Can we use send(sock, "", 0) to reenable FD_WRITE, perhaps?
> > 
> > Can't it just be assumed that the socket is _always_ writeable _unless_ the last send() failed?
> > In other words, try to always send() if it did not fail before.  If it did, only send() after
> > FD_WRITE was returned in the event mask.
> 
> You're looking from the application perspective, but as the underlying
> library we don't have the application under control.  The application
> can rightfully expect POSIX-like behaviour from select(2), and *that*
> means, it can expect select(2) to return a socket as non-writable if the
> internal buffer is full, *before* it calls send:
> 
>   while (...)
>     {
>       /* send as long as we can, otherwise do another job in the meantime */
>       while (select (..., <zero timeout>))
> 	send (<blocking>);
>       <do something else>
>     }

No, wait.

I just realized that this isn't correct.  While select indicates that
data can be written, it doesn't indicate how much data can be written.
I. e., if select returns, and there's only buffer space for 10 bytes,
and the send call tries to send 100 bytes, it *will* block, unless the
socket is non-blocking and returns EAGAIN.

The testcase my patch was based on called a poll/write loop on a
socketpair without changing the socket to non-blocking before.  At the
time I didn't even realize that it's actually not a good test, d'oh.


Corinna

Re: scp stalls on uploading in cygwin 3.5 current master.

[Takashi Yano]

Hi Corinna,

On Fri, 25 Aug 2023 12:50:56 +0200
Corinna Vinschen wrote:
> On Aug 25 17:48, Takashi Yano via Cygwin wrote:
> > This did not help. I looked into this deeper and noticed that:
> > 1) _win32_select() sometimes returns 0.
> > 2) If _win32_select() returns 0, WaitForMultipleObjects(..., INFINITE)
> >    is called in thread_socket().
> > 3) WaitForMultipleObjects() sometimes does not return for FD_WRITE
> >    for unknown reason.
> > This causes the stall.
> 
> So the situation is that the network event handling returned FD_WRITE,
> because it always returns FD_WRITE as long as a non-blocking send()
> function didn't explicitely fail due to buffer overrun.
> 
> However, _win32_select will notice that the buffer is full, so it
> does not return 1, but 0.  I e., the socket is not ready for writing.
> 
> Now you're saying that it's possible that the following WFMO will
> never return?
> 
> That would mean that the FD_WRITE event won't be triggered again because
> it already *had* been triggered and the only way to re-enable it is to
> call one of the send() functions (see
> https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-wsaeventselect)
> 
> I don't have an answer to this problem yet.
> 
> Can we use send(sock, "", 0) to reenable FD_WRITE, perhaps?

Your idea seems to work. The following patch looks to solve the issue.
Is it supposed to be any side effect()?

diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 7b9473849..443f95e68 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -1824,8 +1824,16 @@ thread_socket (void *arg)
     {
       for (select_record *s = si->start; (s = s->next); )
 	if (s->startup == start_thread_socket)
-	  if (peek_socket (s, false))
-	    event = true;
+	  {
+	    if (peek_socket (s, false))
+	      event = true;
+	    else if (s->write_selected)
+	      {
+		/* Retrigger WSA socket event */
+		fhandler_socket_wsock *fh = (fhandler_socket_wsock *) s->fh;
+		fh->write ("", 0);
+	      }
+	  }
       if (!event)
 	for (int i = 0; i < si->num_w4; i += MAXIMUM_WAIT_OBJECTS)
 	  switch (WaitForMultipleObjects (MIN (si->num_w4 - i,


-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

RE: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Lavrentiev, Anton (NIH/NLM/NCBI) [C]]

> While select indicates that
> data can be written, it doesn't indicate how much data can be written.
> I. e., if select returns, and there's only buffer space for 10 bytes,
> and the send call tries to send 100 bytes, it *will* block, unless the
> socket is non-blocking and returns EAGAIN.

IIRC, if there's space for 10 bytes in the internal buffer, send(100) will return 10, whether or not the socket is blocking.
EAGAIN is only returned when nothing at all can be written to a non-blocking socket; or send() blocks (when blocking).

Anton Lavrentiev
Contractor NIH/NLM/NCBI

> -----Original Message-----
> From: Cygwin <cygwin-bounces+lavr=ncbi.nlm.nih.gov@cygwin.com> On Behalf Of Corinna
> Vinschen via Cygwin
> Sent: Friday, August 25, 2023 9:19 AM
> To: cygwin@cygwin.com
> Cc: Corinna Vinschen <corinna-cygwin@cygwin.com>
> Subject: Re: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.
>
> On Aug 25 14:23, Corinna Vinschen via Cygwin wrote:
> > On Aug 25 12:08, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin wrote:
> > > > I don't have an answer to this problem yet.
> > > >
> > > > Can we use send(sock, "", 0) to reenable FD_WRITE, perhaps?
> > >
> > > Can't it just be assumed that the socket is _always_ writeable _unless_ the last
> send() failed?
> > > In other words, try to always send() if it did not fail before.  If it did, only
> send() after
> > > FD_WRITE was returned in the event mask.
> >
> > You're looking from the application perspective, but as the underlying
> > library we don't have the application under control.  The application
> > can rightfully expect POSIX-like behaviour from select(2), and *that*
> > means, it can expect select(2) to return a socket as non-writable if the
> > internal buffer is full, *before* it calls send:
> >
> >   while (...)
> >     {
> >       /* send as long as we can, otherwise do another job in the meantime */
> >       while (select (..., <zero timeout>))
> >       send (<blocking>);
> >       <do something else>
> >     }
>
> No, wait.
>
> I just realized that this isn't correct.  While select indicates that
> data can be written, it doesn't indicate how much data can be written.
> I. e., if select returns, and there's only buffer space for 10 bytes,
> and the send call tries to send 100 bytes, it *will* block, unless the
> socket is non-blocking and returns EAGAIN.
>
> The testcase my patch was based on called a poll/write loop on a
> socketpair without changing the socket to non-blocking before.  At the
> time I didn't even realize that it's actually not a good test, d'oh.
>
>
> Corinna
>
>
>
> --
> Problem reports:
> https://cygwin.com/problems.ht
> ml&data=05%7C01%7Clavr%40ncbi.nlm.nih.gov%7Ca0231ac386e44a5dbd0008dba56e384b%7C14b77578977
> 342d58507251ca2dc2b06%7C0%7C0%7C638285665032412610%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=jPh5i6OE5cYkS1zZ
> 6ntr2t2c%2B%2BJgl6Gfp6YqVcypj98%3D&reserved=0
> FAQ:
> https://cygwin.com/faq/
> =05%7C01%7Clavr%40ncbi.nlm.nih.gov%7Ca0231ac386e44a5dbd0008dba56e384b%7C14b77578977342d585
> 07251ca2dc2b06%7C0%7C0%7C638285665032412610%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
> JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=o4iHOc%2FbDPmql%2BxjqVl
> %2FgIM0FYW%2FN9%2Bsenjy1JqT9oE%3D&reserved=0
> Documentation:
> https://cygwin.com/docs.html
> ata=05%7C01%7Clavr%40ncbi.nlm.nih.gov%7Ca0231ac386e44a5dbd0008dba56e384b%7C14b77578977342d
> 58507251ca2dc2b06%7C0%7C0%7C638285665032412610%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=i0y2q1tMqjh78EeJhBOh
> swPgYENictSU7V2XnMEFZ5k%3D&reserved=0
> Unsubscribe info:
> https://cygwin.com/ml/#uns
> ubscribe-
> simple&data=05%7C01%7Clavr%40ncbi.nlm.nih.gov%7Ca0231ac386e44a5dbd0008dba56e384b%7C14b7757
> 8977342d58507251ca2dc2b06%7C0%7C0%7C638285665032412610%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=bT4vlE52NW83
> P4xFNOeu2wTSWq9k1WdZ573j03JKHHA%3D&reserved=0
> CAUTION: This email originated from outside of the organization. Do not click links or
> open attachments unless you recognize the sender and are confident the content is safe.

Re: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Corinna Vinschen]

On Aug 25 23:27, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin wrote:
> > While select indicates that
> > data can be written, it doesn't indicate how much data can be written.
> > I. e., if select returns, and there's only buffer space for 10 bytes,
> > and the send call tries to send 100 bytes, it *will* block, unless the
> > socket is non-blocking and returns EAGAIN.
> 
> IIRC, if there's space for 10 bytes in the internal buffer, send(100)
> will return 10, whether or not the socket is blocking.

It doesn't, afaics.  It raises the buffer size until something a bit
less than 512K and then it starts blocking.

> EAGAIN is only returned when nothing at all can be written to a
> non-blocking socket; or send() blocks (when blocking).

This thread is not about send() blocking or returning EAGAIN.  This
is about the behaviour of select(2) and poll(2).


Corinna

RE: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Lavrentiev, Anton (NIH/NLM/NCBI) [C]]

> > Can we use send(sock, "", 0) to reenable FD_WRITE, perhaps?
> 
> Your idea seems to work. The following patch looks to solve the issue.
> Is it supposed to be any side effect()?

IMO you're triggering an undefined (or not well-defined) behavior, because of the murky status
of the byte count of 0.  It may work now, but it may not work in the future...

RETURN VALUE
       On  success,  the  number of bytes written is returned (zero indicates nothing was written).  On error, -1 is returned, and
       errno is set appropriately.

       If count is zero and fd refers to a regular file, then write() may return a failure status if one of the  errors  below  is
       detected.   If no errors are detected, 0 will be returned without causing any other effect.  If count is zero and fd refers
       to a file other than a regular file, the results are not specified.

Anton Lavrentiev
Contractor NIH/NLM/NCBI

P.S.

Think of the effects of "recv(sock, buf, 0)" along the lines: the only valid return values in this
case would be -1 and 0, whereas 0 [usually] means EOF, but would it be really meant in this case?

RE: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Lavrentiev, Anton (NIH/NLM/NCBI) [C]]

> This thread is not about send() blocking or returning EAGAIN.  This
> is about the behaviour of select(2) and poll(2).

I was merely commenting on your note that if select() returned a socket as
writable, and send() writes more than internally allowed, then send() would block.
It wouldn't!  It'd just write the allowable count, and return as a short write,
whether the socket was blocking or not.

select() and poll() whether I/O would block.  By the virtue of this,
a socket, which is in error (except for EAGAIN), is writable exactly
because write()/send() to such a socket would return -1 right away.

Anton Lavrentiev
Contractor NIH/NLM/NCBI

Re: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Corinna Vinschen]

On Aug 26 14:15, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin wrote:
> > This thread is not about send() blocking or returning EAGAIN.  This
> > is about the behaviour of select(2) and poll(2).
> 
> I was merely commenting on your note that if select() returned a socket as
> writable, and send() writes more than internally allowed, then send() would block.
> It wouldn't!  It'd just write the allowable count, and return as a short write,
> whether the socket was blocking or not.

But that's not the situation here.  It's not helpful to get this kind of
comment without it actually referring to the context in question.  We're
coming from a select call which is implemented via Winsock-specific
network events.  Everything else is a followup and a result of this.

> select() and poll() whether I/O would block.  By the virtue of this,
> a socket, which is in error (except for EAGAIN), is writable exactly
> because write()/send() to such a socket would return -1 right away.

And that's not the case her either.


Corinna

Re: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Takashi Yano]

On Sat, 26 Aug 2023 14:08:34 +0000
"Lavrentiev, Anton (NIH/NLM/NCBI) [C]" wrote:
> IMO you're triggering an undefined (or not well-defined) behavior, because of the murky status
> of the byte count of 0.  It may work now, but it may not work in the future...
> 
> RETURN VALUE
>        On  success,  the  number of bytes written is returned (zero indicates nothing was written).  On error, -1 is returned, and
>        errno is set appropriately.
> 
>        If count is zero and fd refers to a regular file, then write() may return a failure status if one of the  errors  below  is
>        detected.   If no errors are detected, 0 will be returned without causing any other effect.  If count is zero and fd refers
>        to a file other than a regular file, the results are not specified.

You seems to reffer Linux man, however, this patch calls
ssize_t fhandler_socket_wsock::write (const void *in_ptr, size_t len)
of cygwin, NOT Linux write().

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

RE: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Lavrentiev, Anton (NIH/NLM/NCBI) [C]]

> You seems to reffer Linux man, however, this patch calls

I was referring to a known behavior.  Your patch gets to call send(s,"",0),
which is technically a write call, and which in this case, falls into an undefined
domain for its argument, and hence, may be expected to change without notice.
That's all I was referring to.

Anton Lavrentiev
Contractor NIH/NLM/NCBI

Re: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Takashi Yano]

On Mon, 28 Aug 2023 13:37:11 +0000
"Lavrentiev, Anton (NIH/NLM/NCBI) [C]" wrote:
> > You seems to reffer Linux man, however, this patch calls
> 
> I was referring to a known behavior.  Your patch gets to call send(s,"",0),
> which is technically a write call, and which in this case, falls into an undefined
> domain for its argument, and hence, may be expected to change without notice.
> That's all I was referring to.

https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-send
  Calling send with a len parameter of zero is permissible and will be treated
  by implementations as successful. In such cases, send will return zero as a
  valid value. For message-oriented sockets, a zero-length transport datagram
  is sent.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

RE: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Lavrentiev, Anton (NIH/NLM/NCBI) [C]]

> For message-oriented sockets, a zero-length transport datagram is sent.

And how is that acceptable?  This will interject a message into some application protocol,
which may not be expected at the application level.

Anton Lavrentiev
Contractor NIH/NLM/NCBI

Re: [EXTERNAL] Re: scp stalls on uploading in cygwin 3.5 current master.

[Takashi Yano]

On Mon, 28 Aug 2023 14:07:44 +0000
"Lavrentiev, Anton (NIH/NLM/NCBI) [C]" wrote:
> > For message-oriented sockets, a zero-length transport datagram is sent.
> 
> And how is that acceptable?  This will interject a message into some application protocol,
> which may not be expected at the application level.

Yeah, due to these undesired behaviours, we decided not to adopt
this patch. Instead, we reverted the original patch.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>