ext/sockets: using accept4 wheneever possible for php_accept_connect … #19268
Conversation
There was a problem hiding this comment.
On first sight this looks fine, but I also found this note in the manpage:
The accept4() function behaves exactly like accept(), but the socket it
returns does not inherit the O_NONBLOCK flag of s; instead, it applies
the following bits set in flags to the returned file descriptor:
ext/sockets/sockets.c
Outdated
| @@ -284,7 +284,12 @@ static bool php_open_listen_sock(php_socket *sock, unsigned short port, int back | |||
| static bool php_accept_connect(php_socket *in_sock, php_socket *out_sock, struct sockaddr *la, socklen_t *la_len) /* {{{ */ | |||
| { | |||
| #if defined(HAVE_ACCEPT4) | |||
| out_sock->bsd_socket = accept4(in_sock->bsd_socket, la, la_len, SOCK_CLOEXEC); | |||
| int flags = SOCK_CLOEXEC; | |||
…helper Haiku now supports it as well now, so we can simplify the workflow a bit for those platforms.
devnexen
force-pushed
the
haiku_accept4_support
branch
from
da1013b to
93bdc8f
Compare
| @@ -283,6 +283,19 @@ static bool php_open_listen_sock(php_socket *sock, unsigned short port, int back | |||
|
|
|||
| static bool php_accept_connect(php_socket *in_sock, php_socket *out_sock, struct sockaddr *la, socklen_t *la_len) /* {{{ */ | |||
| { | |||
| #if defined(HAVE_ACCEPT4) | |||
| int flags = SOCK_CLOEXEC; | |||
| if (!in_sock->blocking) { | |||
There was a problem hiding this comment.
Turns out it's even more complicated:
From https://linux.die.net/man/2/accept4:
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 noninheritance of file status flags and always explicitly set all required flags on the socket returned from accept().
So I guess on BSD you have to inherit the flag and on Linux not, in order to remain consistent?
There was a problem hiding this comment.
we do not create sockets with O_ASYNC in the extension. However it seems to me it s better to set SOCK_NONBLOCK for all platforms
for cross-platform and explicit control, it is always best practice to use accept4() and include the SOCK_NONBLOCK flag if you require a non-blocking socket
There was a problem hiding this comment.
I guess that makes sense. Damn why are the C interfaces always so inconsistent
…helper
Haiku now supports it as well now, so we can simplify the workflow a bit for those platforms.