Re: [PATCH v12 04/20] index-helper: new daemon for caching index and related stuff

On Sun, Jun 26, 2016 at 9:29 PM, David Turner [off-list ref] wrote:

On 06/26/2016 12:27 AM, Duy Nguyen wrote:

quoted

On Sat, Jun 25, 2016 at 9:21 PM, David Turner [off-list ref] wrote:

quoted

On 06/25/2016 10:33 AM, Duy Nguyen wrote:

quoted

quoted

+               /*
+                * 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