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.
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:55 | admin | set | github: 51772 |
| 2011年10月27日 16:45:04 | nvetoshkin | set | messages: + msg146503 |
| 2011年10月27日 16:43:56 | nvetoshkin | set | messages: + msg146502 |
| 2010年10月14日 15:22:38 | pitrou | set | status: open -> closed resolution: fixed messages: + msg118668 stage: needs patch -> resolved |
| 2010年10月13日 21:07:38 | nvetoshkin | set | files:
+ issue7523_py3k_accept4_2.diff messages: + msg118584 |
| 2010年10月13日 21:04:12 | pitrou | set | messages: + msg118583 |
| 2010年10月13日 20:49:51 | nvetoshkin | set | messages: + msg118581 |
| 2010年10月13日 19:41:40 | nvetoshkin | set | files:
+ issue7523_py3k_accept4_1.diff messages: + msg118569 |
| 2010年10月13日 14:12:20 | giampaolo.rodola | set | nosy:
+ giampaolo.rodola |
| 2010年10月12日 21:03:36 | pitrou | set | messages: + msg118473 |
| 2010年10月12日 20:49:13 | nvetoshkin | set | files:
+ issue7523_py3k_accept4.diff messages: + msg118471 |
| 2010年10月12日 19:29:47 | pitrou | set | messages: + msg118463 |
| 2010年10月12日 18:53:27 | nvetoshkin | set | files:
+ issue7523_py3k.diff nosy: + nvetoshkin messages: + msg118457 |
| 2010年07月22日 15:46:41 | pitrou | set | nosy:
+ exarkun messages: + msg111183 components: - IO stage: patch review -> needs patch |
| 2010年07月22日 15:37:16 | BreamoreBoy | set | nosy:
+ BreamoreBoy messages: + msg111182 versions: - Python 2.7 |
| 2010年01月13日 08:32:44 | lekma | set | messages: + msg97699 |
| 2010年01月12日 18:05:51 | pitrou | set | messages: + msg97641 |
| 2010年01月12日 17:53:05 | pitrou | set | messages: + msg97636 |
| 2009年12月20日 09:18:33 | lekma | set | files:
+ Issue7523_3.diff messages: + msg96666 |
| 2009年12月19日 16:09:05 | r.david.murray | set | messages: + msg96609 |
| 2009年12月19日 09:58:45 | lekma | set | messages: + msg96598 |
| 2009年12月18日 11:14:49 | r.david.murray | set | priority: normal nosy: + r.david.murray messages: + msg96559 stage: patch review |
| 2009年12月18日 07:53:00 | lekma | set | messages: + msg96551 |
| 2009年12月17日 13:50:46 | pitrou | set | nosy:
+ pitrou messages: + msg96513 |
| 2009年12月17日 07:24:52 | lekma | set | files:
+ Issue7523_2.diff messages: + msg96504 components: + IO |
| 2009年12月16日 12:31:33 | lekma | set | files:
+ Issue7523.diff keywords: + patch messages: + msg96484 |
| 2009年12月16日 11:00:36 | lekma | create | |