Skip to content

Fix GH-24173: Sockets left in non-blocking mode on Windows - #24174

Open
vibbow wants to merge 4 commits into
php:PHP-8.4from
vibbow:fix-gh24173-win-restore-blocking
Open

vibbow wants to merge 4 commits into
php:PHP-8.4from
vibbow:fix-gh24173-win-restore-blocking

Conversation

@vibbow

@vibbow vibbow commented Oct 7, 2026 •

Copy link
Copy Markdown

Fixes #24173.

php_network_connect_socket() switches the socket to non-blocking mode for the connect, and for a synchronous connect it is supposed to restore the original mode afterwards. On Windows, SET_SOCKET_BLOCKING_MODE() sets save = TRUE, and FIONBIO doesn't write back the previous mode. So RESTORE_SOCKET_BLOCKING_MODE() passed TRUE again, and the socket stayed non-blocking while the stream reported blocked => true. Paths that call recv() without polling first, such as stream_socket_recvfrom() and socket_import_stream() + socket_read(), then failed immediately with WSAEWOULDBLOCK.

Winsock can't query a socket's blocking mode, but all callers of php_network_connect_socket() pass freshly created sockets, which are blocking. So this restores to blocking, which matches the POSIX behavior. Async connects don't call RESTORE, so they are unaffected.

Since Windows has no MSG_DONTWAIT, timed writes relied on the socket being left non-blocking: with a blocking socket, send() in php_sockop_write() could block past the stream timeout. So php_sockop_write() now switches to non-blocking mode for the duration of a timed write on Windows and restores blocking mode afterwards, like xp_ssl.c does. This also fixes timed writes on accepted sockets and after stream_set_blocking($s, true), which were already blocking and hung on Windows before this change.

Tests:

  • gh24173.phpt (Windows only): a socket imported from a client stream is blocking.
  • gh24173_write_timeout.phpt (Windows only): a timed fwrite() to a peer that doesn't read times out, for both a client socket and an accepted socket.

Tested on Windows 11 with an 8.4 NTS x64 build (built with php-windows-builder):

  • fwrite() with stream_set_timeout() or default_socket_timeout times out on client sockets, accepted sockets, and after stream_set_blocking(true).
  • stream_socket_recvfrom() and socket_import_stream() + socket_read() wait for data.
  • The socket is back in blocking mode after a timed write.
  • fread()/fgets() timeouts, feof() and non-blocking writes are unchanged.
  • ext/standard/tests/streams, ext/standard/tests/network and ext/sockets/tests have the same results as before the change.

Also tested on Ubuntu 26.04: the same test directories pass, and behavior is unchanged (the code changes are Windows only).

This is independent of #24172 (GH-24171), which touches the same function.

🤖 Generated with Claude Code

php_network_connect_socket() switches the socket to non-blocking mode
for the connect and restores the original mode afterwards for
synchronous connects. On Windows, SET_SOCKET_BLOCKING_MODE() sets
`save = TRUE` and FIONBIO does not write back the previous mode, so
RESTORE_SOCKET_BLOCKING_MODE() set the socket to non-blocking again.

Winsock cannot query a socket's blocking mode, but all callers pass
freshly created (blocking) sockets, so restore to blocking. This
matches the POSIX behavior.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@devnexen

devnexen commented Oct 9, 2026

Copy link
Copy Markdown
Member

I m pretty sure the fix is incorrect, part of it is MSG_DONTWAIT is 0 on windows. @shivammathur wdyt ?

@vibbow

vibbow commented Oct 9, 2026

Copy link
Copy Markdown
Author

I m pretty sure the fix is incorrect, part of it is MSG_DONTWAIT is 0 on windows. @shivammathur wdyt ?

Yes, you are right.

Do you want me to withdraw this PR, or let Claude give another try?

I'm not really sure what happened inside.

@devnexen

devnexen commented Oct 9, 2026

Copy link
Copy Markdown
Member

I believe the bug is real so it s worth fixing. AI or not however having a real windows expert looking into it is valuable.

@shivammathur

Copy link
Copy Markdown
Member

@vibbow
Yes. Since we don't have MSG_DONTWAIT on Windows, this can make fwrite() hang even with a timeout set.
The PR should also make timed writes use nonblocking mode and test that.

Windows has no MSG_DONTWAIT, so now that the socket is restored to
blocking mode after a synchronous connect, send() could block past the
stream timeout. Switch to non-blocking mode for the duration of a timed
write and restore blocking mode afterwards, as xp_ssl.c does.

This also fixes timed writes on accepted sockets and after
stream_set_blocking(true), which were already blocking on Windows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vibbow

vibbow commented Oct 9, 2026

Copy link
Copy Markdown
Author

@devnexen @shivammathur Thanks. I've updated it as suggested: the socket is still restored to blocking after a synchronous connect, and php_sockop_write() now switches to non-blocking mode on Windows for timed writes and restores blocking mode afterwards. gh24173_write_timeout.phpt covers this for both client and accepted sockets.

On Windows, fwrite() timeouts now work on client sockets, accepted sockets, and after stream_set_blocking(true) (the last two hung before this PR as well). The streams, network and sockets test directories have the same results as before the change, on both Windows and Linux. Details are in the updated description.

vibbow and others added 2 commits October 10, 2026 02:29
A write that times out may still have written part of the chunk, so
fwrite() returns a non-zero length. On macOS this happened on many
iterations, and the test hit the run-tests timeout. Stop at the first
write that reports a timeout instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test still hit the run-tests timeout on macOS, although the fix is
Windows only, so timed writes to a stalled loopback peer don't time out
reliably there either way. The bug and the fix are Windows specific, so
limit the test to Windows like gh24173.phpt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vibbow

vibbow commented Oct 10, 2026

Copy link
Copy Markdown
Author

gh24173_write_timeout.phpt hit the run-tests timeout on MACOS_ARM64_DEBUG_NTS (it passed on all other jobs). The C changes are Windows only, so this looks like existing macOS behavior: a timed fwrite() to a loopback peer that never reads didn't return within 120 s there, even when the test stopped at the first timed-out write. I don't have a Mac to look into it, so I've limited the test to Windows like gh24173.phpt, since the bug and the fix are Windows specific. It may be worth a separate look on macOS.

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