classification
Title: On Mac / BSD sockets returned by accept inherit the parent's FD flags
Type: behavior Stage: resolved
Components: Library (Lib), macOS Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ronaldoussoren Nosy List: Justin.Cappos, bbangert, exarkun, giampaolo.rodola, kristjan.jonsson, loewis, ned.deily, nicdumz, pitrou, ronaldoussoren, rosslagerwall, roysmith, stutzbach
Priority: normal Keywords: patch

Created on 2010-02-22 22:09 by Justin.Cappos, last changed 2011-03-31 11:16 by kristjan.jonsson. This issue is now closed.

Files
File name Uploaded Description Edit
python-nonportable.py Justin.Cappos, 2010-02-22 22:09 an example program that demonstrates the issue. Test script that will fail on Mac / BSD but work on Windows / Linux.
7995_v1.patch rosslagerwall, 2010-12-31 06:04 Patch + test for issue
7995_v2.patch rosslagerwall, 2010-12-31 14:50 Updated patch
7995_v3.patch rosslagerwall, 2011-01-02 17:06 Updated patch to opt-out
nonblock.patch pitrou, 2011-01-02 19:26
nonblock2.patch pitrou, 2011-01-04 14:56
nonblock3.patch pitrou, 2011-01-05 17:07
bug.py kristjan.jonsson, 2011-03-30 18:30 bug demonstration with cpython head revision
defined.patch kristjan.jonsson, 2011-03-31 11:16 suggesteate to whatever review
Repositories containing patches
http://hg.python.org/cpython
Messages (52)
msg99852 - (view) Author: Justin Cappos (Justin.Cappos) Date: 2010-02-22 22:09
Suppose there is a program that has a listening socket that calls accept to obtain new sockets for client connections.   socketmodule.c assumes that these client sockets have timeouts / blocking in the default state for new sockets (which on most systems means the sockets will block).   However, socketmodule.c does not verify the state of the socket object that is returned by the system call accept.

From http://linux.die.net/man/2/accept :
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 behaviour differs from the canonical BSD sockets implementation. Portable programs should not rely on inheritance or non-inheritance of file status flags and always explicitly set all required flags on the socket returned from accept(). 


socketmodule.c does not explicitly set or check these flags for sockets returned by accept.   The attached program will print the following on Linux regardless of whether the settimeout line for s exists or not:

a has timeout: None
O_NONBLOCK is set: False
received: hi



On Mac / BSD, the program will produce the following output when the timeout is set on the listening socket:

a has timeout: None
O_NONBLOCK is set: True
Traceback (most recent call last):
  File "python-nonportable.py", line 39, in <module>
    message = a.recv(1024)
socket.error: (35, 'Resource temporarily unavailable')


When the timeout is removed, the behavior is the same as linux:

a has timeout: None
O_NONBLOCK is set: False
received: hi


Note that the file descriptor problem crops up in odd ways on Mac systems.   It's possible that issue 5154 may be due to this bug.   I am aware of other problems with the socketmodule on Mac and will report them in other tickets.


I believe that this problem can be easily mitigated by always calling fcntl to unset the O_NONBLOCK flag after accept (O_ASYNC should be unset too, for correctness).   I would recommend adding the below code snippet at line 1653 in socketmodule.c (r78335).   The resulting code would look something like this (with '+' in front of the added lines):


'''
#ifdef MS_WINDOWS
        if (newfd == INVALID_SOCKET)
#else
        if (newfd < 0)
#endif
                return s->errorhandler();

+#if defined(__APPLE__) || defined(__OpenBSD__) || defined(__FreeBSD__)
+        int starting_flag;
+        // Unset O_NONBLOCK an O_ASYNC if they are inherited.
+        starting_flag = fcntl(newfd, F_GETFL, 0);
+        starting_flag &= ~(O_NONBLOCK | O_ASYNC);
+        fcntl(newfd, F_SETFL, starting_flag);
+#endif

        /* Create the new object with unspecified family,
           to avoid calls to bind() etc. on it. */
        sock = (PyObject *) new_sockobject(newfd,
                                           s->sock_family,
                                           s->sock_type,
                                           s->sock_proto);
'''

I've tested this patch on my Mac and Linux systems and it seems to work fine.   I haven't had a chance to test on BSD.   Also, I did not test for this problem in Python 3, but I assume it exists there as well and the same fix should be applied.
msg100570 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2010-03-07 09:35
I've removed 2.5 and added 3.2 because there won't be further bugfix releases of 2.5 and the issue also affects 3.2.

IMHO changing this behavior is not a bugfix and is therefore out of scope for 2.6.x, in particular because this change might break code that runs fine on OSX and assumes the current behavior.

The documntation is not entirely clear on the behavior of the accept method, the best I could find is the documentation of the setblocking method: that says that all sockets are blocking by default, that would mean that the result of accept should be a blocking socket.

I'm therefore +1 for this change. I'm setting the 'needs review' flag because I'd like some input from other developers as well.
msg121886 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2010-11-21 08:45
(See also Issue7322)
msg121924 - (view) Author: Roy Smith (roysmith) Date: 2010-11-21 13:36
The answer depends on what the socket module is trying to do.  Is the goal simply to provide a pythonic thin wrapper over the underlying OS interfaces without altering their semantics, or to provide a completely homogeneous abstraction?  Having attempted the latter before, I'm aware of just how difficult a job it can be.

The docs have a big bold note right up top, "Note Some behavior may be platform dependent, since calls are made to the operating system socket APIs".  This is followed up by, "The platform-specific reference material for the various socket-related system calls are also a valuable source of information on the details of socket semantics."

What's not clear, however, is the intent.  If the intent is to hide the platform differences, then the notes in the docs are simply warnings that we're not always successful at doing that.  If so, then exposing the different behaviors of listen/accept is a bug which should be fixed.

Anyway, my personal opinion is that we should consider the current behavior a bug and fix it.  I like the idea of setting all accepted sockets to blocking mode (and documenting it clearly).  I think it is what most people would expect.  I understand that this would break code of people who were relying on the "accept inherits non-blocking mode" behavior on some OS's, but I suspect the number of people who are relying on that behavior is extremely close to zero, and they are relying on a non-portable feature of a specific OS.

I leave it to others to figure out which versions it is reasonable to apply this to.
msg121925 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-21 13:43
It seems to me that it's a reasonable request. There is indeed a bug, since Python uses non-blocking sockets to implement its "timeout" feature, and flags inheritance means Python's view of whether the socket is non-blocking is not in sync with reality on the OS side.

I think the patch should be opt-out rather than opt-in: that code path should probably be enabled for everything but Linux (and Windows?).

Another possibility is to force inheritance of flags, such that an accept()ed socket inherits the timeout settings of its parent (even under Linux).
msg121926 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-21 13:43
Oh, and a test should be added of course.
msg121962 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-21 19:05
> The answer depends on what the socket module is trying to do.  Is the
> goal simply to provide a pythonic thin wrapper over the underlying OS
> interfaces without altering their semantics, or to provide a
> completely homogeneous abstraction?

Most definitely the latter. The method called accept must absolutely
always call the same-named underlying system call, and have no other
side effects, unless
a) the system doesn't provide such a system call, or
b) something else is called, but this behaves *as if* the original
   system call was called, or
c) deviating behavior was explicitly requested from the application.

> The docs have a big bold note right up top, "Note Some behavior may
> be platform dependent, since calls are made to the operating system
> socket APIs".

Having such a note is fine with me; it shouldn't be big and bold,
though. There is a strong opposition to big and bold notes; they
should be only used for really serious problems (such as the likely
risk of data loss).

> Anyway, my personal opinion is that we should consider the current
> behavior a bug and fix it.  I like the idea of setting all accepted
> sockets to blocking mode (and documenting it clearly).

-1.

> I think it is what most people would expect.

Apparently, the designers of BSD thought differently. Remember that
it is them who defined the socket API in the first place, so they
have the right that their design decisions are considered.

We should also take issue10115 into account, which proposes changes
to accept on Linux. One solution might to add optional flags to
accept(), asking for certain behavior variations.
msg121968 - (view) Author: Justin Cappos (Justin.Cappos) Date: 2010-11-21 19:26
> Apparently, the designers of BSD thought differently. Remember that
> it is them who defined the socket API in the first place, so they
> have the right that their design decisions are considered.

I think there is a bit of confusion here.   The 'bug' isn't with different socket semantics on different OSes.   The bug is that the programmer who wrote the wrapper for sockets on Python assumed the OS semantics weren't the BSD style.   


Here is the issue (put plainly):

Python sockets support a notion of timeout  (note this notion is not reflected in the OS socket API).

The python socket implementation of timeouts uses the underlying OS / socket API to provide this by setting the socket to nonblocking and setting a timeout value in a Python object that holds socket info.

This implementation assumes that the OS sets any socket it receives via accept to nonblocking.   (this is a false assumption on BSD)

The end result is that the OS has a nonblocking socket and the Python object thinks it is blocking.   This is why the socket object in Python has timeout=None yet calling fcntl shows the socket is nonblocking.

Calling code paths that handle timeouts and expect the socket to block causes bugs like I described in my code.   This behavior is clearly wrong under any interpretation!


You can debate whether the right patch is to use what I proposed or instead change new Python sockets to inherit the timeout / blocking setting on BSD.   However, what is implemented now is clearly broken.
msg121972 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-21 19:45
> > Anyway, my personal opinion is that we should consider the current
> > behavior a bug and fix it.  I like the idea of setting all accepted
> > sockets to blocking mode (and documenting it clearly).
> 
> -1.
> 
> > I think it is what most people would expect.
> 
> Apparently, the designers of BSD thought differently. Remember that
> it is them who defined the socket API in the first place, so they
> have the right that their design decisions are considered.

We are talking about the timeout feature, which is a Python feature, not
a BSD (or Linux) sockets feature. It should work properly, even if that
means adding some boilerplate around system calls.

> We should also take issue10115 into account, which proposes changes
> to accept on Linux. One solution might to add optional flags to
> accept(), asking for certain behavior variations.

Adding flags to control inheritance could be done. But here the problem
is that the behaviour is buggy even without assuming the API makes any
promise w.r.t. inheritance.
(in other words, while issue10115 is a feature request, this issue is
really a bug)
msg121974 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-21 20:12
> Here is the issue (put plainly):

Unfortunately, I still don't understand it. I go through your plain
elaboration below.

> Python sockets support a notion of timeout  (note this notion is not
> reflected in the OS socket API).

Correct.

> The python socket implementation of timeouts uses the underlying OS /
> socket API to provide this by setting the socket to nonblocking and
> setting a timeout value in a Python object that holds socket info.

Correct.

> This implementation assumes that the OS sets any socket it receives
> via accept to nonblocking.   (this is a false assumption on BSD)

Not true. It doesn't assume that (it doesn't assume the reverse,
either).

> The end result is that the OS has a nonblocking socket and the Python
> object thinks it is blocking.   This is why the socket object in
> Python has timeout=None yet calling fcntl shows the socket is
> nonblocking.

That conclusion is flawed. Python has not associated a timeout with
the socket. It makes no claims as to whether the socket is blocking
or not. So you have created a non-blocking socket without timeout.

I cannot see anything wrong with such a thing. The system explicitly
supports such sockets (and, as you point out, it doesn't even have
the notion of "per-socket timeouts"). And so should Python support
them as well.

> Calling code paths that handle timeouts and expect the socket to
> block causes bugs like I described in my code.   This behavior is
> clearly wrong under any interpretation!

What about my interpretation above?
1
msg121975 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-21 20:16
> We are talking about the timeout feature, which is a Python feature, not
> a BSD (or Linux) sockets feature. It should work properly, even if that
> means adding some boilerplate around system calls.

I agree that the semantics of the socket timeout API is confusing (and I
wish it had not been added in the first place). However, making it work
"better" must not cause breakage to other legitimate applications.
msg121978 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-21 20:24
Le dimanche 21 novembre 2010 à 20:16 +0000, Martin v. Löwis a écrit :
> 
> > We are talking about the timeout feature, which is a Python feature, not
> > a BSD (or Linux) sockets feature. It should work properly, even if that
> > means adding some boilerplate around system calls.
> 
> I agree that the semantics of the socket timeout API is confusing (and I
> wish it had not been added in the first place). However, making it work
> "better" must not cause breakage to other legitimate applications.

Well, I don't think setting a timeout on a listening socket and then
expecting the socket received through accept() to be non-blocking (but
only on BSD) is a legitimate application.
msg121979 - (view) Author: Justin Cappos (Justin.Cappos) Date: 2010-11-21 20:26
>> This implementation assumes that the OS sets any socket it receives
>> via accept to nonblocking.   (this is a false assumption on BSD)
>
> Not true. It doesn't assume that (it doesn't assume the reverse,
> either).

The Python implementation sets timeout=None (which implies that the
underlying socket is blocking).


>> The end result is that the OS has a nonblocking socket and the Python
>> object thinks it is blocking.   This is why the socket object in
>> Python has timeout=None yet calling fcntl shows the socket is
>> nonblocking.
>
> That conclusion is flawed. Python has not associated a timeout with
> the socket. It makes no claims as to whether the socket is blocking
> or not. So you have created a non-blocking socket without timeout.

The problem is that it has.   It has created a new Python socket
object with a specific value for timeout (None), but the underlying
socket is nonblocking.

The docs state that timeout = None makes the socket blocking.
msg121981 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-21 20:36
> Well, I don't think setting a timeout on a listening socket and then
> expecting the socket received through accept() to be non-blocking (but
> only on BSD) is a legitimate application.

Right. But setting the server socket to nonblocking, and then expecting
the connection socket to also be nonblocking might be.
msg121983 - (view) Author: Justin Cappos (Justin.Cappos) Date: 2010-11-21 20:40
> > Well, I don't think setting a timeout on a listening socket and then
> > expecting the socket received through accept() to be non-blocking (but
> > only on BSD) is a legitimate application.
>
>
> Right. But setting the server socket to nonblocking, and then 
> expecting the connection socket to also be nonblocking might be.

Okay sure.   This is fine.   That is why I suggested that if you don't like my patch, one might instead change new Python sockets to inherit the timeout / blocking setting on BSD.

However, I hope we can all agree that having the Python socket object in a different blocking / non-blocking state than the OS socket descriptor is wrong.   This is what needs to be fixed.
msg121984 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-21 20:41
> The Python implementation sets timeout=None (which implies that the
> underlying socket is blocking).

No, it doesn't. A socket may be non-blocking without having a timeout;
that's the socket API (on all systems, not just BSD).

> The problem is that it has.   It has created a new Python socket
> object with a specific value for timeout (None), but the underlying
> socket is nonblocking.
> 
> The docs state that timeout = None makes the socket blocking.

What specific wording are you looking at that makes you believe so?
To me, the phrase

# A socket object can be in one of three modes: blocking, non-blocking,
# or timeout

suggests otherwise.
msg121986 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-21 20:49
> However, I hope we can all agree that having the Python socket object
> in a different blocking / non-blocking state than the OS socket
> descriptor is wrong.   This is what needs to be fixed.

Ok, now I see where your misunderstanding is: you simply can't infer
from calling .gettimeout() whether the socket is blocking or not.
Python does not "think" you can, nor should you. So I don't think
anything needs to be fixed in this respect.
msg121998 - (view) Author: Roy Smith (roysmith) Date: 2010-11-21 21:22
I got into this by starting with Issue7322, which reports a scenario where data is lost using makefile().  The docs for makefile() say, "The socket must be in blocking mode (it can not have a timeout)".  So, we've got published documentation which equates non-blocking mode with a timeout set.
msg122013 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-21 22:57
> I got into this by starting with Issue7322, which reports a scenario
> where data is lost using makefile().  The docs for makefile() say,
> "The socket must be in blocking mode (it can not have a timeout)".
> So, we've got published documentation which equates non-blocking mode
> with a timeout set.

This wording goes back to r54491, from issue882297. Facundo writes

# If the socket is in blocking mode, it can NOT have a timeout.
# I fixed the docs, and made that explicit.

so I don't think he really meant to equate the two. I'm not a native
speaker (of English), so I'll refrain from proposing an alternative
wording.
msg122039 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2010-11-22 00:15
The notes in the documentation under socket.gettimeout() do go into more detail than elsewhere.  But at least one thing there is at best misleading: "Sockets are always created in blocking mode" is, as we've seen, not correct for BSD-ish systems for sockets returned by accept().  So, at a minimum, this section and the documentation for socket.accept() should be expanded, along with any other changes.

http://docs.python.org/py3k/library/socket.html#socket.socket.gettimeout
http://docs.python.org/py3k/library/socket.html#socket.socket.accept
msg122041 - (view) Author: Roy Smith (roysmith) Date: 2010-11-22 00:24
Responding to msg122013:

I think he exactly meant to equate the two.

The original problem described in issue882297 is that the makefile() documentation only stated that the socket could not be in non-blocking mode.  The test case presented didn't appear to set non-blocking mode, it only set a timeout.

The explanation by facundobatista was that setting a timeout inplies putting the socket into non-blocking mode, and the docs were updated to make this "non-blocking <==> timeout" relationship clear.
msg122042 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-22 00:25
> The notes in the documentation under socket.gettimeout() do go into
> more detail than elsewhere.  But at least one thing there is at best
> misleading: "Sockets are always created in blocking mode" is, as we've
> seen, not correct for BSD-ish systems for sockets returned by
> accept().  So, at a minimum, this section and the documentation for
> socket.accept() should be expanded, along with any other changes.

Well, unless someone explains convincingly how the current behaviour is
desireable (rather than misleading and useless), I really think this is
a bug that should be fix (then we can also discuss what the fix should
exactly be).
msg122055 - (view) Author: Justin Cappos (Justin.Cappos) Date: 2010-11-22 01:22
>> The Python implementation sets timeout=None (which implies that the
>> underlying socket is blocking).
>
>No, it doesn't. A socket may be non-blocking without having a timeout;
> that's the socket API (on all systems, not just BSD).

Sure and this happens when the timeout is 0, but None has a different meaning than 0.

>> The problem is that it has.   It has created a new Python socket
>> object with a specific value for timeout (None), but the underlying
>> socket is nonblocking.
>> 
>> The docs state that timeout = None makes the socket blocking.
>
> What specific wording are you looking at that makes you believe so?

Here is the last part of the description of settimeout:

s.settimeout(None) is equivalent to s.setblocking(1)

if you look at setblocking:

Set blocking or non-blocking mode of the socket: if flag is 0, the socket is set to non-blocking, else to blocking mode. 

This seems to imply that timeout = None -> blocking.
msg122096 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-22 07:47
> Well, unless someone explains convincingly how the current behaviour is
> desireable (rather than misleading and useless), I really think this is
> a bug that should be fix (then we can also discuss what the fix should
> exactly be).

So please propose a fix.
msg122097 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2010-11-22 07:54
> Set blocking or non-blocking mode of the socket: if flag is 0, the
> socket is set to non-blocking, else to blocking mode.
> 
> This seems to imply that timeout = None -> blocking.

I see. So gettimeout() should really make a system call, to determine
whether to return 0.0 or None.
msg122133 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-11-22 15:06
> > Well, unless someone explains convincingly how the current behaviour is
> > desireable (rather than misleading and useless), I really think this is
> > a bug that should be fix (then we can also discuss what the fix should
> > exactly be).
> 
> So please propose a fix.

Well, in case you didn't notice, a fix was already proposed :)
(not in proper form, granted, and lacking a test)

At least now you won't argue that this is normal behaviour, so we are
progressing.
msg124954 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2010-12-31 06:04
Attached is a patch (the original one in patch form) against py3k with unit test.

It seems to work well - tested on Linux & FreeBSD.
msg124958 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2010-12-31 09:14
issue1515839 seems to be a duplicate of this one.
msg124965 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-12-31 14:13
Thanks for the patch. Comments/questions:
- please don't use C++-style comments ("//") in C code; some compilers can choke on them
- should the code path be also enabled for netbsd? (or other variants?)
- why does the test silence socket.error on accept()?
- what if fcntl() returns -1? (unlikely I know)
msg124967 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2010-12-31 14:50
From what I coud see, the same applied to NetBSD so I enabled it for NetBSD as well - if there are any other OSes that need it enabled, they can be added.

The updated patch checks for fcntl() failing and simply leaves the socket as is if it fails.

The test silenced socket.error() just because the other tests did like testSetBlocking, testInitNonBlocking, testAccept, etc. I updated it to remove this.
msg125064 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-02 16:06
I've tried the patch under OpenSolaris and the test fails (EAGAIN),
meaning that accept() semantics there are the same as under BSD:

======================================================================
ERROR: testInheritFlags (test.test_socket.NonBlockingTCPTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/antoine/vbox/py3k/cc/Lib/test/test_socket.py", line 983,
in testInheritFlags
    message = conn.recv(len(MSG))
socket.error: [Errno 11] Resource temporarily unavailable

I think the code path in the patch should be opt-out rather than opt-in:
that is, it should be executed if HAVE_FCNTL, O_NONBLOCK are defined,
and if not under Linux.
(and I don't think O_ASYNC is useful here, is it?)
msg125066 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-02 17:06
OK try this one, it's now opt-out.
msg125078 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-02 19:26
After further testing, it turns out that Windows exhibits BSD-like behaviour too. So instead of complicating the flag-setting code again, I suggest an alternative of doing it in the Python wrapper. Patch attached.
msg125135 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-02 23:52
I like the logic of Antoine's patch much better (basing this on whether the listening socket had a timeout). I wonder whether it's correct though if there is a defaulttimeout: shouldn't it then leave the timeout on the socket instead?
msg125218 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-03 19:34
> I like the logic of Antoine's patch much better (basing this on
> whether the listening socket had a timeout). I wonder whether it's
> correct though if there is a defaulttimeout: shouldn't it then leave
> the timeout on the socket instead?

Indeed, so I'll try to make the patch better.
msg125276 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-04 01:37
Issue 976613 (duplicate) has a very simple patch, will see if it's sufficient.
msg125331 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-04 14:56
A new patch combining Ross' test with the simple approach from issue 976613. Works under Linux, OpenSolaris and Windows.
msg125372 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-04 22:10
I think this patch (nonblock2.patch) is wrong. If I have a non-blocking server socket on *BSD, and do accept, with no default timeout: IIUC, under the patch, I will get a blocking connection socket. However, according to the operating system API, I'm entitled to get a non-blocking socket (i.e. O_NONBLOCK must be inherited across accept).
msg125373 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-04 22:29
> I think this patch (nonblock2.patch) is wrong. If I have a
> non-blocking server socket on *BSD, and do accept, with no default
> timeout: IIUC, under the patch, I will get a blocking connection
> socket. However, according to the operating system API, I'm entitled
> to get a non-blocking socket (i.e. O_NONBLOCK must be inherited across
> accept).

Well, either the defaulttimeout should have the priority over the parent
socket's settings (your argument in msg125135), or it shouldn't. I'm
fine with both, but I think any more complicated combination would end
up puzzling for the user :)
msg125375 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-04 22:31
> Antoine Pitrou <pitrou@free.fr> added the comment:
> 
> > I think this patch (nonblock2.patch) is wrong. If I have a
> > non-blocking server socket on *BSD, and do accept, with no default
> > timeout: IIUC, under the patch, I will get a blocking connection
> > socket. However, according to the operating system API, I'm entitled
> > to get a non-blocking socket (i.e. O_NONBLOCK must be inherited across
> > accept).
> 
> Well, either the defaulttimeout should have the priority over the parent
> socket's settings (your argument in msg125135), or it shouldn't. I'm
> fine with both, but I think any more complicated combination would end
> up puzzling for the user :)

I would add that, since flags inheritance through accept() is
platform-dependent while the default timeout is a well-defined Python
feature, I would lean slightly towards applying the default timeout.
msg125379 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-04 22:51
> Well, either the defaulttimeout should have the priority over the parent
> socket's settings (your argument in msg125135), or it shouldn't. I'm
> fine with both, but I think any more complicated combination would end
> up puzzling for the user :)

Applying the default timeout if set is fine, and I agree it should have
precedence. In case no default timeout is configured, the platform
semantics should prevail. This is tradition in Python: when we define
some behavior (i.e. default timeout), we are entitled to consistently
define all aspects of it. In the other places we use the platform
semantics (leaning towards standards such as POSIX in case the platform
offers alternative semantics).
msg125437 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-05 17:07
Ok, here is a patch which prefers the default timeout (if set) over fixing of inherited flags. Tested under Linux, Windows, OpenSolaris.
msg125453 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-01-05 18:59
> Ok, here is a patch which prefers the default timeout (if set) over fixing of inherited flags. Tested under Linux, Windows, OpenSolaris.

This patch looks fine to me. Please also update the accept documentation
to explain the situation (new socket gets
default timeout if given, else is blocking if server socket
has timeout, else inherits flags according to system defaults).

I then think that gettimeout is also incorrect: it really
fetch the O_NONBLOCK flag from the socket.
msg125471 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-05 21:21
Ok, I committed the patch in r87768 and overhauled the timeout docs in r87769. I'm not sure this should be backported (because of the very slight behaviour change), so I'm closing.
msg132591 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2011-03-30 13:58
Please see issue 11721 where I was commenting on the same.

I don't think the documentation makes it clear that socket.gettimeout() can be incorrect (i.e. return None when the socket is non-blocking).

I also don't think there is a portable way to detect the NBIO attribute of a socket, so we still have a case of socket.gettimeout() not accurately reflecting the blocking state of the socket.  If people don't think it is a bug, then this fact should be documented.

I personally think that this logic should go into socketmodule.c where the rest of the timeout logic sits, rather than be exctracted like this out into the python world.

Finally, IMHO this is a bug that should be backported as far back as possible.

Cheers!
msg132592 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-30 15:40
> I also don't think there is a portable way to detect the NBIO attribute 
> of a socket, so we still have a case of socket.gettimeout() not 
> accurately reflecting the blocking state of the socket

Which case?

> I personally think that this logic should go into socketmodule.c where 
> the rest of the timeout logic sits, rather than be exctracted like this 
> out into the python world.

I think practicality beats purity. If putting it in socket.py ends up producing bugs, we'll have to consider moving it to the C extension. Until then, the Python code has the advantage of being clear and concise.
msg132593 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2011-03-30 16:15
socket.defaulttimeout(None)
s = socket.socket()
s.settimeout(0) #nonblocking
s.bind()
s2, a = s.accept()
print s2.gettimeout() #prints ´none´, meaning blocking
s2.receive(10) #raises EWOULDBLOCK error, since internally it is non-blocking

I don't agree with practicality vs. purity, particularly when trying to understand the timeout logic.  Most of the timeout logic is implemented in c and never touched by python, in init_sockobject().  But then you tack on extra logic in socket.py, in what is even a socket object wrapper.  This means that any module that uses the "pure" _socket.socket object, such as C extensions, will not get the "correct" behaviour.
msg132594 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-30 16:20
> socket.defaulttimeout(None)
> s = socket.socket()
> s.settimeout(0) #nonblocking
> s.bind()
> s2, a = s.accept()
> print s2.gettimeout() #prints ´none´, meaning blocking
> s2.receive(10) #raises EWOULDBLOCK error, since internally it is non-blocking

Could you post working Python 3 code which demonstrates the issue on
3.3?

> This means that any module that uses the "pure" _socket.socket object,
> such as C extensions, will not get the "correct" behaviour.

Using undocumented implementation details (such as the _socket module)
is, AFAIK, unsupported.

Anyway, if you want the code to be changed, please propose a patch so
that it can be judged on its own merits (together with tests that
demonstrate the improvement, if any).
msg132598 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2011-03-30 17:03
I'm confused by the patch (ed0259230611).  The patch comment and the NEWS item state "the returned socket is now always non-blocking" but the code change adds "sock.setblocking(True)".
msg132601 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2011-03-30 18:30
Antoine, absolument.  Please see attached file bug.py

As for a different patch, we should agree what behaviour should be expected.  I don't think it is possible to rely on some platform specific behaviour.  This is because it is in general not possible to query the blokcking state of a socket.  Instead we should simply define it for python, and in accordance to established tradition, namely that defaulttimeout prevails.

Btw, defaulttimeout(None) doesn't mean that that there is no default, it means that the default is "blocking."
msg132602 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-03-30 18:54
> Antoine, absolument.  Please see attached file bug.py

Ah, thanks, I see. But this is not caused by this issue.
It seems to come from 12442ac3f7dd.

> Instead we should simply define it for python, and in accordance to
> established tradition, namely that defaulttimeout prevails.

That would be the second behaviour change in two versions. I think we
could/should simply fix the advertised behaviour, as in
http://docs.python.org/dev/library/socket.html#timeouts-and-the-accept-method

> Btw, defaulttimeout(None) doesn't mean that that there is no default,
> it means that the default is "blocking."

Sure, but generally it means that the user didn't *set* a default at
all.
msg132650 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2011-03-31 11:16
I can't see how that patch has anything to do with it.  The problem has been present since at least 2.5.  Your patch fixed it for timeout>0.0 but left the 0.0 case still broken.

It comes from these lines in init_sockobject:
{
     s->sock_timeout = defaulttimeout;
     if (defaulttimeout >= 0.0)
         internal_setblocking(s, 0);
}
This code assumes that the socket's internal state is always blocking, and so only switches it into the non-blocking state if required.

Now, you fixed the 'accept' case with your socket.py patch, but you left out the case where accept_socket.gettimeout() == 0.0.  In other words, you fixed only accept_socket.gettimeout() > 0.0

We can fix it to be as advertized, except that the second part is not possible: 
"if the listening socket is in non-blocking mode, whether the socket     returned by accept() is in blocking or non-blocking mode is operating system-dependent. If you want to ensure cross-platform behaviour, it is recommended you manually override this setting."
The reason is that it is impossible to _ask_ the socket whether it is blocking or non-blocking and therefore, accepted_socket.gettimeout() will not be truthful.

imho, we should just simplify this rule and have the accepted socket inherit the accept socket's properties if defaulttimeout() == None, and not make this special case.  I'll prepare a demonstration patch.

I've added a suggested patch.
p.s. My comments about fixing things in socketmodule.c were only half correct.  I didn't realize that accept had been broken into _accept and socket() in py3k.
History
Date User Action Args
2011-03-31 11:16:29kristjan.jonssonsetfiles: + defined.patch
hgrepos: + hgrepo14
messages: + msg132650
2011-03-30 18:54:43pitrousetmessages: + msg132602
2011-03-30 18:30:33kristjan.jonssonsetfiles: + bug.py

messages: + msg132601
2011-03-30 17:03:51stutzbachsetnosy: + stutzbach
messages: + msg132598
2011-03-30 16:20:45pitrousetmessages: + msg132594
2011-03-30 16:15:25kristjan.jonssonsetmessages: + msg132593
2011-03-30 15:40:53pitrousetmessages: + msg132592
2011-03-30 14:00:36kristjan.jonssonlinkissue11721 superseder
2011-03-30 13:58:54kristjan.jonssonsetnosy: + kristjan.jonsson
messages: + msg132591
2011-01-05 21:21:51pitrousetstatus: open -> closed

versions: - Python 3.1, Python 2.7
keywords: - needs review
nosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125471
resolution: fixed
stage: resolved
2011-01-05 19:00:00loewissetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125453
2011-01-05 17:07:18pitrousetfiles: + nonblock3.patch
nosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125437
2011-01-04 22:51:14loewissetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125379
2011-01-04 22:31:45pitrousetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125375
2011-01-04 22:29:49pitrousetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125373
2011-01-04 22:10:05loewissetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125372
2011-01-04 14:56:21pitrousetfiles: + nonblock2.patch
nosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125331
2011-01-04 01:37:02pitrousetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125276
2011-01-04 01:35:34pitroulinkissue976613 superseder
2011-01-03 19:34:29pitrousetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125218
2011-01-02 23:52:35loewissetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125135
2011-01-02 19:26:43pitrousetfiles: + nonblock.patch
nosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125078
2011-01-02 17:06:46rosslagerwallsetfiles: + 7995_v3.patch
nosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125066
2011-01-02 16:06:35pitrousetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg125064
2010-12-31 14:50:48rosslagerwallsetfiles: + 7995_v2.patch
nosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg124967
2010-12-31 14:13:43pitrousetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg124965
2010-12-31 09:32:01ned.deilylinkissue1515839 superseder
2010-12-31 09:14:52rosslagerwallsetnosy: loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall
messages: + msg124958
2010-12-31 06:04:53rosslagerwallsetfiles: + 7995_v1.patch

nosy: + rosslagerwall
messages: + msg124954

keywords: + patch
2010-11-22 15:06:46pitrousetmessages: + msg122133
2010-11-22 07:54:53loewissetmessages: + msg122097
2010-11-22 07:47:00loewissetmessages: + msg122096
2010-11-22 01:22:12Justin.Cappossetmessages: + msg122055
2010-11-22 00:25:04pitrousetmessages: + msg122042
2010-11-22 00:24:30roysmithsetmessages: + msg122041
2010-11-22 00:15:10ned.deilysetmessages: + msg122039
2010-11-21 22:57:17loewissetmessages: + msg122013
2010-11-21 21:22:47roysmithsetmessages: + msg121998
2010-11-21 20:49:27loewissetmessages: + msg121986
2010-11-21 20:41:40loewissetmessages: + msg121984
2010-11-21 20:40:32Justin.Cappossetmessages: + msg121983
2010-11-21 20:36:31loewissetmessages: + msg121981
2010-11-21 20:26:55Justin.Cappossetmessages: + msg121979
2010-11-21 20:24:37pitrousetmessages: + msg121978
2010-11-21 20:16:03loewissetmessages: + msg121975
2010-11-21 20:12:33loewissetmessages: + msg121974
2010-11-21 19:45:42pitrousetmessages: + msg121972
2010-11-21 19:26:16Justin.Cappossetmessages: + msg121968
2010-11-21 19:05:55loewissetmessages: + msg121962
2010-11-21 13:43:57pitrousetmessages: + msg121926
2010-11-21 13:43:10pitrousetnosy: + pitrou

messages: + msg121925
versions: + Python 3.1, - Python 2.6
2010-11-21 13:38:30pitrousetnosy: + exarkun
2010-11-21 13:36:17roysmithsetnosy: + roysmith
messages: + msg121924
2010-11-21 08:45:55ned.deilysetnosy: + ned.deily
messages: + msg121886
2010-03-07 09:35:47ronaldoussorensetkeywords: + needs review

messages: + msg100570
versions: + Python 3.2, - Python 2.5
2010-02-22 22:09:41Justin.Capposcreate