Skip to content

ext/sockets: using accept4 wheneever possible for php_accept_connect … #19268

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

Merged
merged 1 commit into from
Jul 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ext/sockets/config.m4
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ PHP_ARG_ENABLE([sockets],
[Enable sockets support])])

if test "$PHP_SOCKETS" != "no"; then
AC_CHECK_FUNCS([hstrerror if_nametoindex if_indextoname sockatmark])
AC_CHECK_FUNCS([hstrerror if_nametoindex if_indextoname sockatmark accept4])
AC_CHECK_HEADERS([sys/sockio.h linux/filter.h linux/if_packet.h linux/if_ether.h linux/udp.h])
AC_DEFINE([HAVE_SOCKETS], [1],
[Define to 1 if the PHP extension 'sockets' is available.])
Expand Down
16 changes: 15 additions & 1 deletion ext/sockets/sockets.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

@nielsdos nielsdos Jul 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

@devnexen devnexen Jul 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that makes sense. Damn why are the C interfaces always so inconsistent

flags |= SOCK_NONBLOCK;
}

out_sock->bsd_socket = accept4(in_sock->bsd_socket, la, la_len, flags);

if (IS_INVALID_SOCKET(out_sock)) {
PHP_SOCKET_ERROR(out_sock, "unable to accept incoming connection", errno);
return 0;
}
#else
out_sock->bsd_socket = accept(in_sock->bsd_socket, la, la_len);

if (IS_INVALID_SOCKET(out_sock)) {
Expand All @@ -292,7 +305,7 @@ static bool php_accept_connect(php_socket *in_sock, php_socket *out_sock, struct

#if !defined(PHP_WIN32)
/**
* accept4 could had been used but not all platforms support it (e.g. Haiku, solaris < 11.4, ...)
* for fewer and fewer platforms not supporting accept4 syscall we use fcntl instead,
* win32, not having any concept of child process, has no need to address it.
*/
int mode;
Expand All @@ -310,6 +323,7 @@ static bool php_accept_connect(php_socket *in_sock, php_socket *out_sock, struct
return 0;
}
}
#endif
#endif

out_sock->error = 0;
Expand Down