* Questions about select for sockets @ 2021-04-03 18:16 Ken Brown 2021-04-06 14:20 ` Corinna Vinschen 0 siblings, 1 reply; 10+ messages in thread From: Ken Brown @ 2021-04-03 18:16 UTC (permalink / raw) To: cygwin-devel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Questions about select for sockets 2021-04-03 18:16 Questions about select for sockets Ken Brown @ 2021-04-06 14:20 ` Corinna Vinschen 2021-04-06 14:33 ` Corinna Vinschen 0 siblings, 1 reply; 10+ messages in thread From: Corinna Vinschen @ 2021-04-06 14:20 UTC (permalink / raw) To: cygwin-developers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Questions about select for sockets 2021-04-06 14:20 ` Corinna Vinschen @ 2021-04-06 14:33 ` Corinna Vinschen 2021-04-06 15:44 ` Ken Brown 0 siblings, 1 reply; 10+ messages in thread From: Corinna Vinschen @ 2021-04-06 14:33 UTC (permalink / raw) To: cygwin-developers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Questions about select for sockets 2021-04-06 14:33 ` Corinna Vinschen @ 2021-04-06 15:44 ` Ken Brown 2021-04-06 16:28 ` Corinna Vinschen 0 siblings, 1 reply; 10+ messages in thread From: Ken Brown @ 2021-04-06 15:44 UTC (permalink / raw) To: cygwin-developers 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Questions about select for sockets 2021-04-06 15:44 ` Ken Brown @ 2021-04-06 16:28 ` Corinna Vinschen 2021-04-06 17:37 ` Ken Brown 0 siblings, 1 reply; 10+ messages in thread From: Corinna Vinschen @ 2021-04-06 16:28 UTC (permalink / raw) To: cygwin-developers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Questions about select for sockets 2021-04-06 16:28 ` Corinna Vinschen @ 2021-04-06 17:37 ` Ken Brown 2021-04-06 18:24 ` Corinna Vinschen 0 siblings, 1 reply; 10+ messages in thread From: Ken Brown @ 2021-04-06 17:37 UTC (permalink / raw) To: cygwin-developers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Questions about select for sockets 2021-04-06 17:37 ` Ken Brown @ 2021-04-06 18:24 ` Corinna Vinschen 2021-04-06 19:36 ` Corinna Vinschen 0 siblings, 1 reply; 10+ messages in thread From: Corinna Vinschen @ 2021-04-06 18:24 UTC (permalink / raw) To: cygwin-developers 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"); } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Questions about select for sockets 2021-04-06 18:24 ` Corinna Vinschen @ 2021-04-06 19:36 ` Corinna Vinschen 2021-04-07 14:04 ` Ken Brown 0 siblings, 1 reply; 10+ messages in thread From: Corinna Vinschen @ 2021-04-06 19:36 UTC (permalink / raw) To: cygwin-developers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Questions about select for sockets 2021-04-06 19:36 ` Corinna Vinschen @ 2021-04-07 14:04 ` Ken Brown 2021-04-07 15:31 ` Corinna Vinschen 0 siblings, 1 reply; 10+ messages in thread From: Ken Brown @ 2021-04-07 14:04 UTC (permalink / raw) To: cygwin-developers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Questions about select for sockets 2021-04-07 14:04 ` Ken Brown @ 2021-04-07 15:31 ` Corinna Vinschen 0 siblings, 0 replies; 10+ messages in thread From: Corinna Vinschen @ 2021-04-07 15:31 UTC (permalink / raw) To: cygwin-developers 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-04-07 15:31 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-03 18:16 Questions about select for sockets Ken Brown 2021-04-06 14:20 ` Corinna Vinschen 2021-04-06 14:33 ` Corinna Vinschen 2021-04-06 15:44 ` Ken Brown 2021-04-06 16:28 ` Corinna Vinschen 2021-04-06 17:37 ` Ken Brown 2021-04-06 18:24 ` Corinna Vinschen 2021-04-06 19:36 ` Corinna Vinschen 2021-04-07 14:04 ` Ken Brown 2021-04-07 15:31 ` Corinna Vinschen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).