Skip to content

Conversation

@bukka
Copy link
Member

@bukka bukka commented Nov 3, 2025

This adds so_keepalive, tcp_keepidle, tcp_keepintvl and tcp_keepcnt stream socket context options that are used to set their upper case C macro variants by setsockopt function.

The test requires sockets extension and just tests that the values are being set. This is because a real test would be slow and difficult to show that those options really work due to how they work internally.

@lucasnetau
Copy link
Contributor

lucasnetau commented Nov 4, 2025

Is it possible please to also have TCP_USER_TIMEOUT constant added to sockets and made available to be set in the context along with this change? Helps in detecting half closed connections and goes hand in hand with KEEPALIVE

https://blog.cloudflare.com/when-tcp-sockets-refuse-to-die/

This is how I currently need to do it, being able to provide it via context would be a big benefit (For example I need to hack around in ReactPHP/Socket to be able to set this flag because the stream is not easily accessible but context can be provided)

if (!defined('TCP_USER_TIMEOUT')) {
     define("TCP_USER_TIMEOUT", 18);
}
$internalSocket = socket_import_stream($stream);
socket_setopt($internalSocket, SOL_TCP, TCP_USER_TIMEOUT, 15000); //ms

@bukka
Copy link
Member Author

bukka commented Nov 5, 2025

Yeah that could be added but probs in different PR.

#else
#define PHP_STREAM_XPORT_IS_UNIX(_stream) false
#endif
#define PHP_STREAM_XPORT_IS_UDP(_stream) (_stream->ops == &php_stream_udp_socket_ops)
Copy link
Member Author

Choose a reason for hiding this comment

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

I should replace those with php_stream_is

@lucasnetau
Copy link
Contributor

@bukka TCP_USER_TIMEOUT socket option is now available in master since 46e55dd if you would please consider adding a tcp_user_timeout stream socket context option to this PR.

@bukka bukka force-pushed the stream_so_keepalive branch from 51e70d5 to 5dc974b Compare December 30, 2025 13:26
bukka added a commit to bukka/php-src that referenced this pull request Dec 30, 2025
This adds so_keepalive, tcp_keepidle, tcp_keepintvl and tcp_keepcnt
stream socket context options that are used to set their upper case
C macro variants by setsockopt function.

The test requires sockets extension and just tests that the values are
being set. This is because a real test would be slow and difficult to
show that those options really work due to how they work internally.

Closes phpGH-20381
@bukka
Copy link
Member Author

bukka commented Dec 30, 2025

@lucasnetau I have been looking to TCP_USER_TIMEOUT and I can see how it can be useful. However, as far as I see, there are no similar options in other platforms (Windows, FreeBSD, MacOS) so I don't think we should directly expose it in streams. I would say that using socket_import_stream might be better here as one can check if the TCP_USER_TIMEOUT is available and then have the right expectation. Generally I think that the platform specific options should go only to socket extension.

This adds so_keepalive, tcp_keepidle, tcp_keepintvl and tcp_keepcnt
stream socket context options that are used to set their upper case
C macro variants by setsockopt function.

The test requires sockets extension and just tests that the values are
being set. This is because a real test would be slow and difficult to
show that those options really work due to how they work internally.

Closes phpGH-20381
@bukka bukka force-pushed the stream_so_keepalive branch from 5dc974b to 0fd8aae Compare December 30, 2025 15:53
@bukka bukka closed this in 9582d8e Dec 30, 2025
@bukka bukka merged commit 0fd8aae into php:master Dec 30, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants