On Sun, Jun 26, 2016 at 9:29 PM, David Turner <novalis@xxxxxxxxxxx> wrote: > On 06/26/2016 12:27 AM, Duy Nguyen wrote: >> >> On Sat, Jun 25, 2016 at 9:21 PM, David Turner <novalis@xxxxxxxxxxx> wrote: >>> >>> On 06/25/2016 10:33 AM, Duy Nguyen wrote: >>>>> >>>>> >>>>> + /* >>>>> + * Our connection to the client is blocking since a >>>>> client >>>>> + * can always be killed by SIGINT or similar. >>>>> + */ >>>>> + set_socket_blocking_flag(client_fd, 0); >>>> >>>> >>>> >>>> Out of curiosity, do we really need this? I thought default behavior >>>> was always blocking (and checked linux kernel, it seemed to agree with >>>> me). Maybe for extra safety because other OSes may default to >>>> something else? >>> >>> >>> >>> Yes -- see this bug report for details: >>> https://bugs.python.org/issue7995 >>> >> >> I think we should refer to this issue in the comment block right >> before set_socket_blocking_flag() call. Imagine a year from now, I may >> read the code, decide this code is useless and try to remove it. > > > Assuming that we do keep this (see Eric Wong's note), I do not think we need > a comment. It is documented in the man page for accept[1], and it is the > reader's responsibility to understand standard POSIX APIs. > > > > [1] "On Linux, the new socket returned by accept() does not inherit file > status flags such as O_NONBLOCK and O_ASYNC from the listening socket. This > behavior differs from the canonical BSD sockets implementation." But we do not ever set O_NONBLOCK on listening socket, and the default should be no O_NONBLOCK at socket creation even on BSDs, right? If we know where this O_NONBLOCK comes from then we can clear it at that place (maybe with #if BSD to emphasize) instead of after accept(). The bug report actually confuses me because "timeout is set" then O_NONBLOCK is also set, but I don't know exactly what "timeout is set" is, on poll()? I'll check out socketmodule.c later. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html