Repository navigation
Fixed bug GH-22981 (unix socket connect fails with EAGAIN when listen backlog is full) - #23669
shoutoutuoadi325 wants to merge 2 commits into
Conversation
…ten backlog is full) php_network_connect_socket() runs the connect in non-blocking mode so the timeout can be honoured. On a unix domain socket, connect() then returns EAGAIN as soon as the listen backlog is full, instead of waiting for a slot like a blocking connect would. Retry while that is the case, bounded by the connect timeout.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved issues remain in transient-error handling, timeout-bounded retries, and timeout regression coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes Unix-domain socket connection failures when the listen backlog is full by retrying transient connection errors.
Changes:
- Adds bounded retry handling for Unix socket connections.
- Adds GH-22981 regression coverage.
- Documents the fix in
NEWS.
File summaries
| File | Description |
|---|---|
NEWS |
Documents the bug fix. |
main/network.c |
Implements Unix socket connection retries. |
ext/standard/tests/streams/gh22981.phpt |
Tests delayed backlog recovery. |
Review details
Suppressed comments (2)
ext/standard/tests/streams/gh22981.phpt:64
- Please add coverage for a backlog that remains full until the client timeout expires. This test only exercises eventual success—the server starts accepting after 200 ms and every client has a 5-second timeout—so it would not catch a retry loop that ignores or exceeds the supplied timeout.
for ($i = 0; $i < 20; $i++) {
$conn = stream_socket_client('unix://' . $socketPath, $errno, $errstr, 5);
main/network.c:359
- On platforms where
EWOULDBLOCKdiffers fromEAGAIN, a non-blocking UNIX-domainconnect()can reportEWOULDBLOCKfor this transient full-backlog condition. This guard then falls through to the hard-error path, so the fix remains platform-dependent; use the existingPHP_IS_TRANSIENT_ERROR(error)macro, which covers both values.
if (!asynchronous && error == EAGAIN && addr->sa_family == AF_UNIX) {
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The pause between attempts is capped to the time the caller asked for, so a very small (or zero) timeout is no longer overstretched by the 10ms slice. EWOULDBLOCK is now retried as well for the systems where it differs from EAGAIN, and the test also covers a backlog that never drains: the client has to fail once its connect timeout expires.
|
Quoted below: written with Claude, an LLM, as CONTRIBUTING.md asks us to mark. The builds and test runs behind it are on my machine; the commands to repeat them are under the fold.
|
Fixes GH-22981.
php_network_connect_socket() runs the connect with the socket in
non-blocking mode so it can honour the timeout. The flip side is that
connect() to a unix domain socket returns EAGAIN as soon as the listen
backlog fills up, while a blocking connect would wait for a slot. Short
bursts of connections (PDO over a unix socket in the report) then fail
with "Resource temporarily unavailable".
Retry the connect while it keeps returning EAGAIN, bounded by the
connect timeout if one was given, matching what a blocking connect
does. Everything else (other errors, async connects, TCP) is unchanged.
gh22981.phpt runs a server with backlog=1 that starts accepting late:
connect #3 fails before the patch, all 20 succeed after. It also runs a
server that never accepts and expects the client to fail once its
connect timeout expires.