Questions about select for sockets

Questions about select for sockets

[Ken Brown]

Some time ago I did a preliminary implementation of select for AF_UNIX sockets 
on the topic/af_unix branch.  I copied some things from the 
fhandler_socket_wsock case without really understanding them.  I'm taking a 
second look now, and would like to clean this up.  I'll start with just a few 
questions, but there may be more to follow.

1. What's the purpose of 'except_on_write'?  AFAICS, its only use is in 
set_bits, to indicate that the readfds entry should be set in case of a failed 
connect.  But shouldn't that entry always be set if there's a failed connect 
since a read won't block?

2. In that same place in set_bits where UNIX_FD_SET (me->fd, readfds) is called, 
shouldn't ready be incremented?

3. In fhandler_socket_wsock::select_write where except_on_write is set, 
except_ready is also set.  Why is that done there?  It seems to me that this is 
redundant, because the same setting of except_ready is done in 
fhandler_socket_wsock::select_except.

4. Why do we set

   except_ready = saw_shutdown_write () || saw_shutdown_read () ?

I can't find any documentation that says that a shutdown should be considered an 
exceptional condition.  On the other hand, POSIX does say, "Other circumstances 
under which a socket may be considered to have an exceptional condition pending 
are protocol-specific and implementation-defined."  So maybe there's some 
Cygwin-specific reason for doing this?

Thanks.

Ken

Re: Questions about select for sockets

[Corinna Vinschen]

On Apr  3 14:16, Ken Brown via Cygwin-developers wrote:
> Some time ago I did a preliminary implementation of select for AF_UNIX
> sockets on the topic/af_unix branch.  I copied some things from the
> fhandler_socket_wsock case without really understanding them.  I'm taking a
> second look now, and would like to clean this up.  I'll start with just a
> few questions, but there may be more to follow.
> 
> 1. What's the purpose of 'except_on_write'?  AFAICS, its only use is in
> set_bits, to indicate that the readfds entry should be set in case of a
> failed connect.  But shouldn't that entry always be set if there's a failed
> connect since a read won't block?

Just looking this up in W. Richard Steven's book, and from what I can
tell you're right.  Basically, if a SO_ERROR type socket error occured,
the socket is marked for writing and reading.

except_on_write (the name is older and was originally used incorrectly)
is only set if we really *are* connecting asynchronously at the time
select is called.  Only then the check for the connect state makes sense.

Also, if the app is calling async connect and then select, the successful
connection is indicated as ready for writing.

So *typically*, this code is doing the right thing.  However, *iff* the
application actually calls select only checking ready for reading, then
the socket error in case of a failing select would be lost.  Not sure
how useful this scenario is, but probably it wouldn't hurt to set the fd
as ready for reading even if there's no check on ready for writing...

> 2. In that same place in set_bits where UNIX_FD_SET (me->fd, readfds) is
> called, shouldn't ready be incremented?

Uh oh, yes.  Thanks for catching!

> 3. In fhandler_socket_wsock::select_write where except_on_write is set,
> except_ready is also set.  Why is that done there?  It seems to me that this
> is redundant, because the same setting of except_ready is done in
> fhandler_socket_wsock::select_except.
> 
> 4. Why do we set
> 
>   except_ready = saw_shutdown_write () || saw_shutdown_read () ?
> 
> I can't find any documentation that says that a shutdown should be
> considered an exceptional condition.  On the other hand, POSIX does say,
> "Other circumstances under which a socket may be considered to have an
> exceptional condition pending are protocol-specific and
> implementation-defined."  So maybe there's some Cygwin-specific reason for
> doing this?

Nope, this is old (and wrong) cruft.  Neither Steven's book nor the
Linux man pages, nor testing on Linux imply that ready for exception
is used to indicate anything other than out-of-band data.  This should
be fixed.


Thanks,
Corinna

Re: Questions about select for sockets

[Corinna Vinschen]

On Apr  6 16:20, Corinna Vinschen wrote:
> On Apr  3 14:16, Ken Brown via Cygwin-developers wrote:
> > 4. Why do we set
> > 
> >   except_ready = saw_shutdown_write () || saw_shutdown_read () ?
> > 
> > I can't find any documentation that says that a shutdown should be
> > considered an exceptional condition.  On the other hand, POSIX does say,
> > "Other circumstances under which a socket may be considered to have an
> > exceptional condition pending are protocol-specific and
> > implementation-defined."  So maybe there's some Cygwin-specific reason for
> > doing this?
> 
> Nope, this is old (and wrong) cruft.  Neither Steven's book nor the
> Linux man pages, nor testing on Linux imply that ready for exception
> is used to indicate anything other than out-of-band data.  This should
> be fixed.

We may also have to change the saw_shutdown_read/saw_shutdown_write
handling.  I checked this on Linux and what happens is:

  After shutdown (fd, SHUT_RD), the socket is ready for reading and writing
  After shutdown (fd, SHUT_RDWR), the socket is ready for reading and writing
  After shutdown (fd, SHUT_WR), the socket is ready for writing only.


Corinna

Re: Questions about select for sockets

[Ken Brown]

On 4/6/2021 10:33 AM, Corinna Vinschen wrote:
> On Apr  6 16:20, Corinna Vinschen wrote:
>> On Apr  3 14:16, Ken Brown via Cygwin-developers wrote:
>>> 4. Why do we set
>>>
>>>    except_ready = saw_shutdown_write () || saw_shutdown_read () ?
>>>
>>> I can't find any documentation that says that a shutdown should be
>>> considered an exceptional condition.  On the other hand, POSIX does say,
>>> "Other circumstances under which a socket may be considered to have an
>>> exceptional condition pending are protocol-specific and
>>> implementation-defined."  So maybe there's some Cygwin-specific reason for
>>> doing this?
>>
>> Nope, this is old (and wrong) cruft.  Neither Steven's book nor the
>> Linux man pages, nor testing on Linux imply that ready for exception
>> is used to indicate anything other than out-of-band data.  This should
>> be fixed.
> 
> We may also have to change the saw_shutdown_read/saw_shutdown_write
> handling.  I checked this on Linux and what happens is:
> 
>    After shutdown (fd, SHUT_RD), the socket is ready for reading and writing

This seems surprising to me.  Is it really the shutdown that caused it to be 
ready for writing in your test, or was it ready for writing anyway (e.g., 
because the relevant buffer was empty)?

Ken

>    After shutdown (fd, SHUT_RDWR), the socket is ready for reading and writing
>    After shutdown (fd, SHUT_WR), the socket is ready for writing only.
> 
> 
> Corinna
> 

Re: Questions about select for sockets

[Corinna Vinschen]

On Apr  6 11:44, Ken Brown wrote:
> On 4/6/2021 10:33 AM, Corinna Vinschen wrote:
> > On Apr  6 16:20, Corinna Vinschen wrote:
> > > On Apr  3 14:16, Ken Brown via Cygwin-developers wrote:
> > > > 4. Why do we set
> > > > 
> > > >    except_ready = saw_shutdown_write () || saw_shutdown_read () ?
> > > > 
> > > > I can't find any documentation that says that a shutdown should be
> > > > considered an exceptional condition.  On the other hand, POSIX does say,
> > > > "Other circumstances under which a socket may be considered to have an
> > > > exceptional condition pending are protocol-specific and
> > > > implementation-defined."  So maybe there's some Cygwin-specific reason for
> > > > doing this?
> > > 
> > > Nope, this is old (and wrong) cruft.  Neither Steven's book nor the
> > > Linux man pages, nor testing on Linux imply that ready for exception
> > > is used to indicate anything other than out-of-band data.  This should
> > > be fixed.
> > 
> > We may also have to change the saw_shutdown_read/saw_shutdown_write
> > handling.  I checked this on Linux and what happens is:
> > 
> >    After shutdown (fd, SHUT_RD), the socket is ready for reading and writing
> 
> This seems surprising to me.  Is it really the shutdown that caused it to be
> ready for writing in your test, or was it ready for writing anyway (e.g.,
> because the relevant buffer was empty)?

I guess so, too.  How to make sure the socket isn't ready for writing
without going to great lengths?


Corinna

Re: Questions about select for sockets

[Ken Brown]

On 4/6/2021 12:28 PM, Corinna Vinschen wrote:
> On Apr  6 11:44, Ken Brown wrote:
>> On 4/6/2021 10:33 AM, Corinna Vinschen wrote:
>>> On Apr  6 16:20, Corinna Vinschen wrote:
>>>> On Apr  3 14:16, Ken Brown via Cygwin-developers wrote:
>>>>> 4. Why do we set
>>>>>
>>>>>     except_ready = saw_shutdown_write () || saw_shutdown_read () ?
>>>>>
>>>>> I can't find any documentation that says that a shutdown should be
>>>>> considered an exceptional condition.  On the other hand, POSIX does say,
>>>>> "Other circumstances under which a socket may be considered to have an
>>>>> exceptional condition pending are protocol-specific and
>>>>> implementation-defined."  So maybe there's some Cygwin-specific reason for
>>>>> doing this?
>>>>
>>>> Nope, this is old (and wrong) cruft.  Neither Steven's book nor the
>>>> Linux man pages, nor testing on Linux imply that ready for exception
>>>> is used to indicate anything other than out-of-band data.  This should
>>>> be fixed.
>>>
>>> We may also have to change the saw_shutdown_read/saw_shutdown_write
>>> handling.  I checked this on Linux and what happens is:
>>>
>>>     After shutdown (fd, SHUT_RD), the socket is ready for reading and writing
>>
>> This seems surprising to me.  Is it really the shutdown that caused it to be
>> ready for writing in your test, or was it ready for writing anyway (e.g.,
>> because the relevant buffer was empty)?
> 
> I guess so, too.  How to make sure the socket isn't ready for writing
> without going to great lengths?

I guess you could have a subprocess write to the socket in a loop, so that its 
buffer will quickly fill up and a further write will block.

Ken

Re: Questions about select for sockets

[Corinna Vinschen]

On Apr  6 13:37, Ken Brown wrote:
> On 4/6/2021 12:28 PM, Corinna Vinschen wrote:
> > On Apr  6 11:44, Ken Brown wrote:
> > > On 4/6/2021 10:33 AM, Corinna Vinschen wrote:
> > > > We may also have to change the saw_shutdown_read/saw_shutdown_write
> > > > handling.  I checked this on Linux and what happens is:
> > > > 
> > > >     After shutdown (fd, SHUT_RD), the socket is ready for reading and writing
> > > 
> > > This seems surprising to me.  Is it really the shutdown that caused it to be
> > > ready for writing in your test, or was it ready for writing anyway (e.g.,
> > > because the relevant buffer was empty)?
> > 
> > I guess so, too.  How to make sure the socket isn't ready for writing
> > without going to great lengths?
> 
> I guess you could have a subprocess write to the socket in a loop, so that
> its buffer will quickly fill up and a further write will block.

Yeah, I was trying to minimize work, but I now lazily created a blocking
server in the same process with a non-blocking client, calling send(2)
until it fails.

And now everything is as expected.  SHUT_RD -> ready for reading,
SHUT_WR -> ready for writing, SHUT_RDWR -> ready for both.

I attached my STC, for completeness.  Call with an argument
0 (== SHUT_RD), 1 (== SHUT_WR), or 2 (== SHUT_RDWR).


Corinna


#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/select.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <errno.h>

int
sel (int fd, const char *when)
{
  fd_set r, w, e;
  int ret;
  struct timeval tv;

  printf ("%s\n", when);
  FD_ZERO (&r);
  FD_ZERO (&w);
  FD_ZERO (&e);
  FD_SET (fd, &r);
  FD_SET (fd, &w);
  FD_SET (fd, &e);
  tv.tv_sec = 1;
  tv.tv_usec = 0;
  errno = 0;
  ret = select (fd + 1, &r, &w, &e, &tv);
  printf ("select: %d", ret);
  if (select < 0)
    printf ("%d <%s>\n", errno, strerror (errno));
  putchar ('\n');
  if (FD_ISSET (fd, &r))
    printf ("ready for reading\n");
  if (FD_ISSET (fd, &w))
    printf ("ready for writing\n");
  if (FD_ISSET (fd, &r) || FD_ISSET (fd, &w))
    {
      int err, len;

      len = sizeof err;
      getsockopt (fd, SOL_SOCKET, SO_ERROR, &err, &len);
      printf ("SO_ERROR: %d <%s>\n", err, strerror (err));
    }
  if (FD_ISSET (fd, &e))
    printf ("ready for exception\n");
  putchar ('\n');
}

int sfd;

int
server_1 ()
{
  struct sockaddr_in in;

  sfd = socket (AF_INET, SOCK_STREAM, 0);
  if (sfd < 0)
    {
      printf ("SRV socket: %d <%s>\n", errno, strerror (errno));
      return -1;
    }
  in.sin_family = AF_INET;
  in.sin_port = htons (65444);
  in.sin_addr.s_addr = inet_addr ("127.0.0.1");
  if (bind (sfd, (const struct sockaddr *) &in, sizeof in) < 0)
    {
      printf ("SRV bind: %d <%s>\n", errno, strerror (errno));
      return -1;
    }
  if (listen (sfd, 5) < 0)
    {
      printf ("SRV listen: %d <%s>\n", errno, strerror (errno));
      return -1;
    }
  return 0;
}

int
server_2 ()
{
  if (accept (sfd, NULL, NULL) < 0)
    {
      printf ("SRV accept: %d <%s>\n", errno, strerror (errno));
      return -1;
    }
  return 0;
}

char buf[65536];

int
main (int argc, char **argv)
{
  int fd, arg;
  struct sockaddr_in in;

  fd = socket (AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
  if (fd < 0)
    {
      printf ("socket: %d <%s>\n", errno, strerror (errno));
      return 1;
    }

  if (server_1 ())
    return 2;

  in.sin_family = AF_INET;
  in.sin_port = htons (65444);
  in.sin_addr.s_addr = inet_addr ("127.0.0.1");
  sel (fd, "Pre connect");
  if (connect (fd, (const struct sockaddr *) &in, sizeof in) != 0
      && errno != EINPROGRESS)
    {
      printf ("connect: %d <%s>\n", errno, strerror (errno));
      return 3;
    }

  if (server_2 () < 0)
    return 4;

  sel (fd, "Post shutdown");

  /* write until buffer full */
  while (send (fd, buf, sizeof buf, 0) > 0)
    ;

  sel (fd, "Pre shutdown");
  arg = argc > 1 ? atoi (argv[1]) : SHUT_RDWR;
  if (shutdown (fd, argc > 1 ? atoi (argv[1]) : SHUT_RDWR) < 0)
    {
      printf ("shutdown: %d <%s>\n", errno, strerror (errno));
      return 4;
    }
  sel (fd, "Post shutdown");
}

Re: Questions about select for sockets

[Corinna Vinschen]

On Apr  6 20:24, Corinna Vinschen wrote:
> On Apr  6 13:37, Ken Brown wrote:
> > On 4/6/2021 12:28 PM, Corinna Vinschen wrote:
> > > On Apr  6 11:44, Ken Brown wrote:
> > > > On 4/6/2021 10:33 AM, Corinna Vinschen wrote:
> > > > > We may also have to change the saw_shutdown_read/saw_shutdown_write
> > > > > handling.  I checked this on Linux and what happens is:
> > > > > 
> > > > >     After shutdown (fd, SHUT_RD), the socket is ready for reading and writing
> > > > 
> > > > This seems surprising to me.  Is it really the shutdown that caused it to be
> > > > ready for writing in your test, or was it ready for writing anyway (e.g.,
> > > > because the relevant buffer was empty)?
> > > 
> > > I guess so, too.  How to make sure the socket isn't ready for writing
> > > without going to great lengths?
> > 
> > I guess you could have a subprocess write to the socket in a loop, so that
> > its buffer will quickly fill up and a further write will block.
> 
> Yeah, I was trying to minimize work, but I now lazily created a blocking
> server in the same process with a non-blocking client, calling send(2)
> until it fails.
> 
> And now everything is as expected.  SHUT_RD -> ready for reading,
> SHUT_WR -> ready for writing, SHUT_RDWR -> ready for both.
> 
> I attached my STC, for completeness.  Call with an argument
> 0 (== SHUT_RD), 1 (== SHUT_WR), or 2 (== SHUT_RDWR).

I pushed a patch to handle this better.  Please have a look.


Thanks,
Corinna

Re: Questions about select for sockets

[Ken Brown]

On 4/6/2021 3:36 PM, Corinna Vinschen wrote:
> On Apr  6 20:24, Corinna Vinschen wrote:
>> On Apr  6 13:37, Ken Brown wrote:
>>> On 4/6/2021 12:28 PM, Corinna Vinschen wrote:
>>>> On Apr  6 11:44, Ken Brown wrote:
>>>>> On 4/6/2021 10:33 AM, Corinna Vinschen wrote:
>>>>>> We may also have to change the saw_shutdown_read/saw_shutdown_write
>>>>>> handling.  I checked this on Linux and what happens is:
>>>>>>
>>>>>>      After shutdown (fd, SHUT_RD), the socket is ready for reading and writing
>>>>>
>>>>> This seems surprising to me.  Is it really the shutdown that caused it to be
>>>>> ready for writing in your test, or was it ready for writing anyway (e.g.,
>>>>> because the relevant buffer was empty)?
>>>>
>>>> I guess so, too.  How to make sure the socket isn't ready for writing
>>>> without going to great lengths?
>>>
>>> I guess you could have a subprocess write to the socket in a loop, so that
>>> its buffer will quickly fill up and a further write will block.
>>
>> Yeah, I was trying to minimize work, but I now lazily created a blocking
>> server in the same process with a non-blocking client, calling send(2)
>> until it fails.
>>
>> And now everything is as expected.  SHUT_RD -> ready for reading,
>> SHUT_WR -> ready for writing, SHUT_RDWR -> ready for both.
>>
>> I attached my STC, for completeness.  Call with an argument
>> 0 (== SHUT_RD), 1 (== SHUT_WR), or 2 (== SHUT_RDWR).
> 
> I pushed a patch to handle this better.  Please have a look.

You forgot this:

--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -528,6 +528,7 @@ set_bits (select_record *me, fd_set *readfds, fd_set *writefds,
           if (!me->read_ready && me->read_selected
               && sock->connect_state () == connect_failed)
             UNIX_FD_SET (me->fd, readfds);
+         ready++;
         }
        ready++;
      }

Otherwise it looks good to me, to the extent that I can judge.  I'm not at all 
familiar with the Winsock events.

Ken

Re: Questions about select for sockets

[Corinna Vinschen]

On Apr  7 10:04, Ken Brown wrote:
> On 4/6/2021 3:36 PM, Corinna Vinschen wrote:
> > On Apr  6 20:24, Corinna Vinschen wrote:
> > > On Apr  6 13:37, Ken Brown wrote:
> > > > On 4/6/2021 12:28 PM, Corinna Vinschen wrote:
> > > > > On Apr  6 11:44, Ken Brown wrote:
> > > > > > On 4/6/2021 10:33 AM, Corinna Vinschen wrote:
> > > > > > > We may also have to change the saw_shutdown_read/saw_shutdown_write
> > > > > > > handling.  I checked this on Linux and what happens is:
> > > > > > > 
> > > > > > >      After shutdown (fd, SHUT_RD), the socket is ready for reading and writing
> > > > > > 
> > > > > > This seems surprising to me.  Is it really the shutdown that caused it to be
> > > > > > ready for writing in your test, or was it ready for writing anyway (e.g.,
> > > > > > because the relevant buffer was empty)?
> > > > > 
> > > > > I guess so, too.  How to make sure the socket isn't ready for writing
> > > > > without going to great lengths?
> > > > 
> > > > I guess you could have a subprocess write to the socket in a loop, so that
> > > > its buffer will quickly fill up and a further write will block.
> > > 
> > > Yeah, I was trying to minimize work, but I now lazily created a blocking
> > > server in the same process with a non-blocking client, calling send(2)
> > > until it fails.
> > > 
> > > And now everything is as expected.  SHUT_RD -> ready for reading,
> > > SHUT_WR -> ready for writing, SHUT_RDWR -> ready for both.
> > > 
> > > I attached my STC, for completeness.  Call with an argument
> > > 0 (== SHUT_RD), 1 (== SHUT_WR), or 2 (== SHUT_RDWR).
> > 
> > I pushed a patch to handle this better.  Please have a look.
> 
> You forgot this:
> 
> --- a/winsup/cygwin/select.cc
> +++ b/winsup/cygwin/select.cc
> @@ -528,6 +528,7 @@ set_bits (select_record *me, fd_set *readfds, fd_set *writefds,
>           if (!me->read_ready && me->read_selected
>               && sock->connect_state () == connect_failed)
>             UNIX_FD_SET (me->fd, readfds);
> +         ready++;
>         }
>        ready++;
>      }

Yeah, I remembered yesterday in the evening but had already forgotten
again this morning :)

Please push this change.


Thanks,
Corinna