New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add SOCK_NONBLOCK and SOCK_CLOEXEC to socket module #51772
Comments
It would be nice to have those. |
First attempt, against trunk. |
better patch (mainly test refactoring). should I provide one against py3k? |
A couple of things:
|
would something like the following be ok?: try: and then: if hasattr(socket, "SOCK_CLOEXEC") and fcntl:
tests.append(CloexecLinuxConstantTest) |
It would be better to use test skipping: (eg: @unittest.SkipUnless |
I didn't know about this feature, thanks for the tip. Now I wonder if it would be better to do it this way: or this way: the second option seems better to me (obvious reason why the test was |
I lean toward the second way, myself. |
this one addresses Antoines's comments (with the help of R. David Murray). |
The patch is basically fine. I'll add a "try .. finally" to the tests. |
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:
Linux has defined accept4() precisely for this purpose:
|
|
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. |
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? |
The accept() issue is the following: the socket created by accept() (in Lib/socket.py) will formally inherit its parent's 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. |
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. |
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. |
Another patch with:
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? |
Here's what strace on FAIL shows (print "in alarm_handler" added) alarm(2) = 0 Here's on OK: alarm(2) = 0 For some reason does another trip through BEGIN_SELECT_LOOP() macro |
Indeed:
+#ifdef HAVE_ACCEPT4 There's a missing curly brace after "if (!timeout)", so accept4() is always called. |
@antoine already found that myself, patched and tested :) thanks! |
Patch committed in r85480, thanks! |
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: |
Sorry, wrong ticket. Right one is 10115 |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: