homepage

When a machine has a newer glibc and an old kernel, accept4 can fail and then Python accept() is unusable. For example:
Traceback (most recent call last):
 File "/home/pybot/buildarea-sid/3.x.klose-debian-sparc/build/Lib/threading.py", line 525, in _bootstrap_inner
 self.run()
 File "/home/pybot/buildarea-sid/3.x.klose-debian-sparc/build/Lib/test/test_asynchat.py", line 37, in run
 conn, client = self.sock.accept()
 File "/home/pybot/buildarea-sid/3.x.klose-debian-sparc/build/Lib/socket.py", line 132, in accept
 fd, addr = self._accept()
socket.error: [Errno 90] Function not implemented
(from http://www.python.org/dev/buildbot/builders/sparc%20Debian%203.x/builds/147/steps/test/logs/stdio )
Improving our configure check wouldn't do a lot of good, since people can reuse Python binaries with different kernels. Perhaps we have to use some kind of runtime fallback.
(Strangely, errno 90 is EMSGSIZE here, and ENOSYS is 38)

Or perhaps we should back out the accept4 change and live with the fact that SOCK_CLOEXEC and SOCK_NONBLOCK can't be inherited by calling accept().

Falling back on result 90 is not that difficult, I think I can submit a patch today. What should be checked ENOSYS or 90?

> Falling back on result 90 is not that difficult, I think I can submit
> a patch today. What should be checked ENOSYS or 90?
That's a rather good question. I will enable some debug printout on the
buildbot.

Ok, 90 is ENOSYS on that buildbot :)

What about exposing accept4() to python level?

> What about exposing accept4() to python level?
That's another possibility, in which case we would first remove the current accept4-calling code in order to fix the buildbot failure.

Weekend is coming, so I can lend a hand in implementing whatever you choose.
Summary:
 * remove accept4() as default and expose it as separate method
 * add runtime fallback if accept4() returns ENOSYS

> Weekend is coming, so I can lend a hand in implementing whatever you
> choose.
I don't have any strong opinion about it, so it's whichever you prefer really :)

Made a proof of concept patch (no doc updates yet). Decided to implement separate accept4() method, cause we have already spent enough time dealing with it and rollback would be pity.

There would need to be some tests.
Also, this last part of the patch looks strange:
@@ -3001,6 +3072,10 @@
 PyErr_SetString(PyExc_ValueError,
 "can't use invalid socket value");
 return -1;
+#ifdef HAVE_ACCEPT4
+ /* These flags are not inherited after accept */
+ type &= ~(SOCK_NONBLOCK & SOCK_CLOEXEC);
+#endif /* HAVE_ACCEPT4 */
What is it meant for? And why does it come right after a "return" statement?

>What is it meant for? And why does it come right after a "return" statement?
@Antoine, if fd was supplied and it was correct (not returned with -1), let's drop flags that can't be inherited.
It's a mistake, at that level we don't know whether we should inherit flags or not... I'm a bit confused...

> That's another possibility, in which case we would first remove the
> current accept4-calling code in order to fix the buildbot failure.
In Python, the lowest layer facing the operating system always directly
exposes the API as-is, without reinterpreting the user's request. Not
following this principle leads exactly to this kind of problem.
So I think .accept() should only call accept(2), and accept4() should
only be called if explicitly requested by the application.
Exposing it as .accept4(flags) is certainly the most straight-forward
way of doing it, but I could also live with .accept(flags) (i.e.
call accept4 if flags are being passed, hoping that no other system
comes up with another accept extension that has a different integer
argument).

I've removed the accept4() call in the meantime (in r85796), so that this issue can be re-classified as a feature request.

Duplicated in issue11157.

@nvetoshkin
Could you update your patch against py3k?
I've got a couple comments (can't login to Rietveld, it's probably due to the change of my tracker account name):
if(flags & ~(SOCK_NONBLOCK | SOCK_CLOEXEC)) {
 PyErr_SetString(PyExc_ValueError,
 "Wrong flag value");
 return NULL;
}
I'm not sure that's necessary:
- accept4() can sanitize its input itself if necessary (will fail with EINVAL)
- if some flag is added, or another OS doesn't use the same flags, it'll fall out of sync
#ifdef HAVE_ACCEPT4
 /* These flags are not inherited after accept */
 type &= ~(SOCK_NONBLOCK & SOCK_CLOEXEC);
#endif /* HAVE_ACCEPT4 */
SOCK_NONBLOCK & SOCK_CLOEXEC == 0, so this doesn't do much.
Second, you should probably reuse what's done in Lib/socket.py for timeout inheritance upon accept (except that here you certainly don't want to set the socket to blocking if SOCK_NONBLOCK flag is passed).
You should add a test for that: have a look at test_pipe2 in Lib/test/test_posix.py.

Yes, I can.
We decided to expose accept4() as another socket method, not accept() replacement?

No one seems to object, and since this approach has been suggested by
Martin and is consistent with the posix module's policy (i.e. a thin
wrapper around the underlying syscall), I guess you can go ahead.

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)

up?

> What value should we assign to sock->sock_timeout if SOCK_NONBLOCK was 
> specified in accept4() call?
The same value as for other non-blocking sockets, IMO.
> And in socket.py should we check as in original accept:
> if getdefaulttimeout() is None and self.gettimeout():
> sock.setblocking(True)
I don't think so, since the whole point of accept4() is to override normal flags creation.

> The same value as for other non-blocking sockets, IMO.
There are three possible values I think:
1. parent's current sock_timeout
2. global default socket timeout
3. 0
Can you please tell which one? I assume it should be number 3.

> > The same value as for other non-blocking sockets, IMO.
> There are three possible values I think:
> 1. parent's current sock_timeout
> 2. global default socket timeout
> 3. 0
> 
> Can you please tell which one? I assume it should be number 3.
Yes (again, IMO).

Can we get this exposed as an os.accept4, and an optional flags parameter to socket.socket.accept?

See the PEP 433 which proposes an unified API to set/unset close-on-exec flag on any function creating a file descriptor.

My implementation of the PEP 433 uses accept4() for socket.accept() if the (new) cloexec parameter is True:
http://hg.python.org/features/pep-433/file/46b7a077ae87/Modules/socketmodule.c#l1961
The code fallbacks to accept() if accept4() fails with ENOSYS. It happens if the glibc version is 2.10 or newer, whereas the Linux kernel is older than 2.6.28. I didn't test the fallback yet.
I see in changeset 12442ac3f7dd (issue #7523) that SOCK_NONBLOCK is also set in the flags parameters of accept4(). I should probably also do that.

Python 3.4 now uses accept4() internally for socket.socket.accept(), the new socket is created non-inheritable. See the PEP 446 for more information (PEP implemented in the issue #18571).