homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: add SOCK_NONBLOCK and SOCK_CLOEXEC to socket module
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, exarkun, giampaolo.rodola, lekma, nvetoshkin, pitrou, r.david.murray
Priority: normal Keywords: patch

Created on 2009年12月16日 11:00 by lekma, last changed 2022年04月11日 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
Issue7523.diff lekma, 2009年12月16日 12:31
Issue7523_2.diff lekma, 2009年12月17日 07:24
Issue7523_3.diff lekma, 2009年12月20日 09:18
issue7523_py3k.diff nvetoshkin, 2010年10月12日 18:53 Ported to py3k patch
issue7523_py3k_accept4.diff nvetoshkin, 2010年10月12日 20:49 Added accept4 via ifdef
issue7523_py3k_accept4_1.diff nvetoshkin, 2010年10月13日 19:41 new test and proper accept4 check
issue7523_py3k_accept4_2.diff nvetoshkin, 2010年10月13日 21:07
Messages (25)
msg96482 - (view) Author: (lekma) * Date: 2009年12月16日 11:00
It would be nice to have those.
See http://udrepper.livejournal.com/20407.html for background.
msg96484 - (view) Author: (lekma) * Date: 2009年12月16日 12:31
First attempt, against trunk.
msg96504 - (view) Author: (lekma) * Date: 2009年12月17日 07:24
better patch (mainly test refactoring).
still against trunk.
should I provide one against py3k?
msg96513 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2009年12月17日 13:50
A couple of things:
- the test shouldn't be marked as Linux-specific since perhaps these
constants will be supported by other systems some day
- when importing fcntl, bear in mind that some systems don't have this
module (e.g. Windows), so you should catch and silence the ImportError
msg96551 - (view) Author: (lekma) * Date: 2009年12月18日 07:52
> - when importing fcntl, bear in mind that some systems don't have this
> module (e.g. Windows), so you should catch and silence the ImportError
would something like the following be ok?:
try:
 import fcntl
except ImportError:
 fcntl = False
and then:
if hasattr(socket, "SOCK_CLOEXEC") and fcntl:
 tests.append(CloexecLinuxConstantTest)
msg96559 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009年12月18日 11:14
It would be better to use test skipping: (eg: @unittest.SkipUnless
before the test class).
msg96598 - (view) Author: (lekma) * Date: 2009年12月19日 09:58
> It would be better to use test skipping: (eg: @unittest.SkipUnless
> before the test class).
I didn't know about this feature, thanks for the tip.
Now I wonder if it would be better to do it this way:
@unittest.SkipUnless(hasattr(socket, "SOCK_CLOEXEC") and fcntl,
"SOCK_CLOEXEC not defined OR module fcntl not available")
or this way:
@unittest.SkipUnless(hasattr(socket, "SOCK_CLOEXEC"), "SOCK_CLOEXEC not
defined")
@unittest.SkipUnless(fcntl, "module fcntl not available")
the second option seems better to me (obvious reason why the test was
skipped), what do you guys think? (it doesn't really matter, I know, but
while we're here...)
msg96609 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009年12月19日 16:09
I lean toward the second way, myself.
msg96666 - (view) Author: (lekma) * Date: 2009年12月20日 09:18
this one addresses Antoines's comments (with the help of R. David Murray).
msg97636 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010年01月12日 17:53
The patch is basically fine. I'll add a "try .. finally" to the tests.
lekma, do you have a real name that we should add to the ACKS file?
msg97641 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010年01月12日 18:05
Looking at it again, there's the question of accept() behaviour. In the original code, it will call internal_setblocking() on the new fd if the listening socket is non-blocking. In the new code, if SOCK_NONBLOCK is defined it will not call any OS function to set the new fd in non-blocking mode.
Here is what the man page has to say:
 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. Portable programs should not rely on inheritance or non-inher‐
 itance of file status flags and always explicitly set all required flags on the socket
 returned from accept().
Linux has defined accept4() precisely for this purpose:
 If flags is 0, then accept4() is the same as accept(). The following values can be bitwise
 ORed in flags to obtain different behavior:
 SOCK_NONBLOCK Set the O_NONBLOCK file status flag on the new open file description.
 Using this flag saves extra calls to fcntl(2) to achieve the same result.
 SOCK_CLOEXEC Set the close-on-exec (FD_CLOEXEC) flag on the new file descriptor. See
 the description of the O_CLOEXEC flag in open(2) for reasons why this may
 be useful.
msg97699 - (view) Author: (lekma) * Date: 2010年01月13日 08:32
> lekma, do you have a real name that we should add to the ACKS file?
no, lekma is fine (that's a nick I carry since childhood)
> Looking at it again, there's the question of accept() behaviour. [...]
Sorry for not having seen that earlier and thanks for pointing it out to me.
Unfortunately, I don't think there is a portable solution. We could restrict the patch to linux 2.6.28 and above (there is already an awful lot of ifdefs in this file).
If you think that's the way to go, I can rework the patch, otherwise I think it's better to close the issue (maybe reopen when bsds catch up, if ever).
Ideas?
msg111182 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010年07月22日 15:37
@Antoine could you respond to msg97699, thanks.
msg111183 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010年07月22日 15:46
> @Antoine could you respond to msg97699, thanks.
Well my comments and the patch itself are outdated now that 2.7 is in bugfix mode. The socket implementation in 3.2 is slightly different, which means the patch should be updated (it isn't necessarily difficult to do so), and the accept() issue should be examined again.
msg118457 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2010年10月12日 18:53
Made an attempt to port lekma's patch to py3k-trunk. No (logical) changes needed.
Don't know about accept4() issue. As I saw in Qt sources, they ifdef'ed CLOEXEC by default on file descriptors. Don't think it's acceptable :) in this particular case. So, @Antoine, what do you say?
msg118463 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010年10月12日 19:29
The accept() issue is the following: the socket created by accept() (in Lib/socket.py) will formally inherit its parent's `type` attribute (including any SOCK_NONBLOCK and SOCK_CLOEXEC flags).
However, the underlying Linux socket is created by the accept() system call, which doesn't inherit flags as mentioned in the aforementioned man page. Therefore, the Python socket gives the wrong information about the socket's real flags.
This can be witnessed quickly:
>>> s = socket.socket(socket.AF_INET, socket.SOCK_STREAM | socket.SOCK_CLOEXEC)
>>> s.bind(("", 0))
>>> s.getsockname()
('0.0.0.0', 34634)
>>> s.listen(5)
>>> c, a = s.accept()
 # Here, just start a "telnet" or "nc" session from another term
>>> import fcntl
>>> fcntl.fcntl(s, fcntl.F_GETFD)
1
>>> fcntl.fcntl(c, fcntl.F_GETFD)
0
>>> fcntl.fcntl(c, fcntl.F_GETFD) & fcntl.FD_CLOEXEC
0
>>> c.type & socket.SOCK_CLOEXEC
524288
The quick solution would be to mask out these flags when creating the Python socket in accept(). A better solution might be to inherit these flags by using the accept4() system call when possible (this is useful especially for SOCK_CLOEXEC, of course).
Apart from that, the patch looks ok, but it would be nice to test that at least the underlying socket is really in non-blocking mode, like is done in NonBlockingTCPTests.testSetBlocking.
msg118471 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2010年10月12日 20:49
Thanks! I can see the problem now, but I think checking should be done like this:
>>> fcntl.fcntl(c, fcntl.F_GETFD) & fcntl.FD_CLOEXEC
0
>>> fcntl.fcntl(s, fcntl.F_GETFD) & fcntl.FD_CLOEXEC
1
and with accept4() call I've got flag set:
>>> fcntl.fcntl(c, fcntl.F_GETFD) & fcntl.FD_CLOEXEC
1
>>> fcntl.fcntl(s, fcntl.F_GETFD) & fcntl.FD_CLOEXEC
1
Don't know how to properly check if accept4 is available.
Second attempt - dropping flags from sock_type should be done on Python level in socket.py and I don't quite like idea to check if SOCK_CLOEXEC is in locals every time.
msg118473 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010年10月12日 21:03
You can check accept4() presence by adding it to the AC_CHECK_FUNCS macro in configure.in around line 2553, and add the corresponding HAVE_ACCEPT4 lines in pyconfig.h.in. Then run "autoconf" and you will have access to a HAVE_ACCEPT4 constant when accept4() exists.
By the way, you shouldn't use C++-style comments ("// ..."), because some C compilers refuse them.
msg118569 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2010年10月13日 19:41
Another patch with:
 - testInitBlocking method
 - no c++ style comments
 - a newly generated configure script (almost 1.5k lines diff)
 - proper accept4 availability check
With this patch I've got 
Traceback (most recent call last):
 File "/home/nekto/workspace/py3k/Lib/test/test_socket.py", line 1564, in testInterruptedTimeout
 foo = self.serv.accept()
socket.timeout: timed out
Can someone test if there's a real regression?
msg118581 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2010年10月13日 20:49
Here's what strace on FAIL shows (print "in alarm_handler" added)
alarm(2) = 0
poll([{fd=3, events=POLLIN}], 1, 5000) = ? ERESTART_RESTARTBLOCK (To be restarted)
--- SIGALRM (Alarm clock) @ 0 (0) ---
rt_sigreturn(0xffffffff) = -1 EINTR (Interrupted system call)
accept4(3, 0x7fffa94c4780, [16], 0) = -1 EAGAIN (Resource temporarily unavailable)
poll([{fd=3, events=POLLIN}], 1, 2999) = 0 (Timeout)
accept4(3, 0x7fffa94c4780, [16], 0) = -1 EAGAIN (Resource temporarily unavailable)
write(1, "in alarm_handler\n", 17) = 17
alarm(0) = 0
Here's on OK:
alarm(2) = 0
poll([{fd=3, events=POLLIN}], 1, 5000) = ? ERESTART_RESTARTBLOCK (To be restarted)
--- SIGALRM (Alarm clock) @ 0 (0) ---
rt_sigreturn(0xffffffff) = -1 EINTR (Interrupted system call)
write(1, "in alarm_handler\n", 17) = 17
alarm(0)
For some reason does another trip through BEGIN_SELECT_LOOP() macro
msg118583 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010年10月13日 21:04
> For some reason does another trip through BEGIN_SELECT_LOOP() macro
Indeed:
 if (!timeout)
+#ifdef HAVE_ACCEPT4
+ /* inherit socket flags and use accept4 call */
+ flags = s->sock_type & (SOCK_CLOEXEC | SOCK_NONBLOCK);
+ newfd = accept4(s->sock_fd, SAS2SA(&addrbuf), &addrlen, flags);
+#else
There's a missing curly brace after "if (!timeout)", so accept4() is always called.
msg118584 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2010年10月13日 21:07
@Antoine already found that myself, patched and tested :) thanks!
msg118668 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010年10月14日 15:22
Patch committed in r85480, thanks!
msg146502 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2011年10月27日 16:43
Started implementing accept4() socket method and stuck on socket object's timeout attribute. What value should we assign to sock->sock_timeout if SOCK_NONBLOCK was specified in accept4() call? And in socket.py should we check as in original accept:
if getdefaulttimeout() is None and self.gettimeout():
 sock.setblocking(True)
msg146503 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2011年10月27日 16:45
Sorry, wrong ticket. Right one is 10115
History
Date User Action Args
2022年04月11日 14:56:55adminsetgithub: 51772
2011年10月27日 16:45:04nvetoshkinsetmessages: + msg146503
2011年10月27日 16:43:56nvetoshkinsetmessages: + msg146502
2010年10月14日 15:22:38pitrousetstatus: open -> closed
resolution: fixed
messages: + msg118668

stage: needs patch -> resolved
2010年10月13日 21:07:38nvetoshkinsetfiles: + issue7523_py3k_accept4_2.diff

messages: + msg118584
2010年10月13日 21:04:12pitrousetmessages: + msg118583
2010年10月13日 20:49:51nvetoshkinsetmessages: + msg118581
2010年10月13日 19:41:40nvetoshkinsetfiles: + issue7523_py3k_accept4_1.diff

messages: + msg118569
2010年10月13日 14:12:20giampaolo.rodolasetnosy: + giampaolo.rodola
2010年10月12日 21:03:36pitrousetmessages: + msg118473
2010年10月12日 20:49:13nvetoshkinsetfiles: + issue7523_py3k_accept4.diff

messages: + msg118471
2010年10月12日 19:29:47pitrousetmessages: + msg118463
2010年10月12日 18:53:27nvetoshkinsetfiles: + issue7523_py3k.diff
nosy: + nvetoshkin
messages: + msg118457

2010年07月22日 15:46:41pitrousetnosy: + exarkun
messages: + msg111183

components: - IO
stage: patch review -> needs patch
2010年07月22日 15:37:16BreamoreBoysetnosy: + BreamoreBoy

messages: + msg111182
versions: - Python 2.7
2010年01月13日 08:32:44lekmasetmessages: + msg97699
2010年01月12日 18:05:51pitrousetmessages: + msg97641
2010年01月12日 17:53:05pitrousetmessages: + msg97636
2009年12月20日 09:18:33lekmasetfiles: + Issue7523_3.diff

messages: + msg96666
2009年12月19日 16:09:05r.david.murraysetmessages: + msg96609
2009年12月19日 09:58:45lekmasetmessages: + msg96598
2009年12月18日 11:14:49r.david.murraysetpriority: normal

nosy: + r.david.murray
messages: + msg96559

stage: patch review
2009年12月18日 07:53:00lekmasetmessages: + msg96551
2009年12月17日 13:50:46pitrousetnosy: + pitrou
messages: + msg96513
2009年12月17日 07:24:52lekmasetfiles: + Issue7523_2.diff

messages: + msg96504
components: + IO
2009年12月16日 12:31:33lekmasetfiles: + Issue7523.diff
keywords: + patch
messages: + msg96484
2009年12月16日 11:00:36lekmacreate

AltStyle によって変換されたページ (->オリジナル) /