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) * |
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) * |
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) * |
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) * |
Date: 2010-11-21 13:43 |
Oh, and a test should be added of course.
|
msg121962 - (view) |
Author: Martin v. Löwis (loewis) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) |
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) |
Date: 2010-12-31 09:14 |
issue1515839 seems to be a duplicate of this one.
|
msg124965 - (view) |
Author: Antoine Pitrou (pitrou) * |
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) |
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) * |
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) |
Date: 2011-01-02 17:06 |
OK try this one, it's now opt-out.
|
msg125078 - (view) |
Author: Antoine Pitrou (pitrou) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) |
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) * |
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) * |
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) * |
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.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:58 | admin | set | github: 52243 |
2011-03-31 11:16:29 | kristjan.jonsson | set | files:
+ defined.patch hgrepos:
+ hgrepo14 messages:
+ msg132650
|
2011-03-30 18:54:43 | pitrou | set | messages:
+ msg132602 |
2011-03-30 18:30:33 | kristjan.jonsson | set | files:
+ bug.py
messages:
+ msg132601 |
2011-03-30 17:03:51 | stutzbach | set | nosy:
+ stutzbach messages:
+ msg132598
|
2011-03-30 16:20:45 | pitrou | set | messages:
+ msg132594 |
2011-03-30 16:15:25 | kristjan.jonsson | set | messages:
+ msg132593 |
2011-03-30 15:40:53 | pitrou | set | messages:
+ msg132592 |
2011-03-30 14:00:36 | kristjan.jonsson | link | issue11721 superseder |
2011-03-30 13:58:54 | kristjan.jonsson | set | nosy:
+ kristjan.jonsson messages:
+ msg132591
|
2011-01-05 21:21:51 | pitrou | set | status: 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:00 | loewis | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125453 |
2011-01-05 17:07:18 | pitrou | set | files:
+ nonblock3.patch nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125437
|
2011-01-04 22:51:14 | loewis | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125379 |
2011-01-04 22:31:45 | pitrou | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125375 |
2011-01-04 22:29:49 | pitrou | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125373 |
2011-01-04 22:10:05 | loewis | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125372 |
2011-01-04 14:56:21 | pitrou | set | files:
+ nonblock2.patch nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125331
|
2011-01-04 01:37:02 | pitrou | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125276 |
2011-01-04 01:35:34 | pitrou | link | issue976613 superseder |
2011-01-03 19:34:29 | pitrou | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125218 |
2011-01-02 23:52:35 | loewis | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125135 |
2011-01-02 19:26:43 | pitrou | set | files:
+ nonblock.patch nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125078
|
2011-01-02 17:06:46 | rosslagerwall | set | files:
+ 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:35 | pitrou | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg125064 |
2010-12-31 14:50:48 | rosslagerwall | set | files:
+ 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:43 | pitrou | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg124965 |
2010-12-31 09:32:01 | ned.deily | link | issue1515839 superseder |
2010-12-31 09:14:52 | rosslagerwall | set | nosy:
loewis, ronaldoussoren, exarkun, roysmith, pitrou, giampaolo.rodola, ned.deily, nicdumz, bbangert, Justin.Cappos, rosslagerwall messages:
+ msg124958 |
2010-12-31 06:04:53 | rosslagerwall | set | files:
+ 7995_v1.patch
nosy:
+ rosslagerwall messages:
+ msg124954
keywords:
+ patch |
2010-11-22 15:06:46 | pitrou | set | messages:
+ msg122133 |
2010-11-22 07:54:53 | loewis | set | messages:
+ msg122097 |
2010-11-22 07:47:00 | loewis | set | messages:
+ msg122096 |
2010-11-22 01:22:12 | Justin.Cappos | set | messages:
+ msg122055 |
2010-11-22 00:25:04 | pitrou | set | messages:
+ msg122042 |
2010-11-22 00:24:30 | roysmith | set | messages:
+ msg122041 |
2010-11-22 00:15:10 | ned.deily | set | messages:
+ msg122039 |
2010-11-21 22:57:17 | loewis | set | messages:
+ msg122013 |
2010-11-21 21:22:47 | roysmith | set | messages:
+ msg121998 |
2010-11-21 20:49:27 | loewis | set | messages:
+ msg121986 |
2010-11-21 20:41:40 | loewis | set | messages:
+ msg121984 |
2010-11-21 20:40:32 | Justin.Cappos | set | messages:
+ msg121983 |
2010-11-21 20:36:31 | loewis | set | messages:
+ msg121981 |
2010-11-21 20:26:55 | Justin.Cappos | set | messages:
+ msg121979 |
2010-11-21 20:24:37 | pitrou | set | messages:
+ msg121978 |
2010-11-21 20:16:03 | loewis | set | messages:
+ msg121975 |
2010-11-21 20:12:33 | loewis | set | messages:
+ msg121974 |
2010-11-21 19:45:42 | pitrou | set | messages:
+ msg121972 |
2010-11-21 19:26:16 | Justin.Cappos | set | messages:
+ msg121968 |
2010-11-21 19:05:55 | loewis | set | messages:
+ msg121962 |
2010-11-21 13:43:57 | pitrou | set | messages:
+ msg121926 |
2010-11-21 13:43:10 | pitrou | set | nosy:
+ pitrou
messages:
+ msg121925 versions:
+ Python 3.1, - Python 2.6 |
2010-11-21 13:38:30 | pitrou | set | nosy:
+ exarkun
|
2010-11-21 13:36:17 | roysmith | set | nosy:
+ roysmith messages:
+ msg121924
|
2010-11-21 08:45:55 | ned.deily | set | nosy:
+ ned.deily messages:
+ msg121886
|
2010-03-07 09:35:47 | ronaldoussoren | set | keywords:
+ needs review
messages:
+ msg100570 versions:
+ Python 3.2, - Python 2.5 |
2010-02-22 22:09:41 | Justin.Cappos | create | |