Fix mysqli_poll timeouts in PHP-WASM - #4170
Draft
chubes4 wants to merge 7 commits into
Draft
Conversation
mysqlnd_poll builds its fd sets through mysqlnd_stream_array_to_fd_set
before select() and reads them back through mysqlnd_stream_array_from_fd_set
after. Both make the same suspending call:
php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL, ...)
Only the from_ side was instrumented, leaving the earlier of the two able
to trap on an uninstrumented frame.
Rebuilding with the poll-path instrumentation cleared the original trap and
exposed the next uninstrumented frame on the same path:
RuntimeError: unreachable
at zend_spprintf
at mysqlnd_mysqlnd_conn_data_tx_begin_pub
at zif_mysqli_begin_transaction
tx_commit_or_rollback_pub was already listed in both name variants;
tx_begin_pub was listed in neither. An async lock-wait cannot be exercised
without a transaction, so the poll path was never reachable in practice.
Rather than walk one trap per rebuild, enumerate every mysqli call the regression makes and check each against ASYNCIFY_ONLY. Missing: zif_mysqli_close, zif_mysqli_report, zif_mysqli_rollback mysqlnd_conn_data_close_pub / send_close_pub (both name variants) zif_mysqli_commit is added alongside rollback: it reaches the same tx_commit_or_rollback_pub method and would trap identically. mysqlnd_com_quit_run was already instrumented while the two methods that reach it were not, so closing an async connection could trap even once the poll itself succeeded.
chubes4
force-pushed
the
fix-4161-mysqli-poll-timeout
branch
from
August 19, 2026 02:04
65f3438 to
a4397a0
Compare
Rebuilt from this branch with the added ASYNCIFY_ONLY instrumentation, pinned to the 8.3.32 patch level already committed here so the binary delta carries the instrumentation change and nothing else. Validated in the same job: the mysqli_poll lock-wait regression passes against these binaries and traps against the previous ones.
Rebuilt from this branch with the added ASYNCIFY_ONLY instrumentation, pinned to the 8.3.32 patch level already committed here so the binary delta carries the instrumentation change and nothing else. Validated in the same job: the mysqli_poll lock-wait regression passes against these binaries and traps against the previous ones.
chubes4
marked this pull request as ready for review
August 19, 2026 02:25
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes PHP-WASM’s mysqli_poll timeout behavior by replacing the existing select() wrapper logic with a shared-deadline polling loop and adding a regression test that reproduces an InnoDB lock wait timeout scenario.
Changes:
- Reworked
__wrap_selectto poll all fds cooperatively against a single monotonic deadline and to filter returned fd-sets by actual readiness. - Added an integration test that reproduces a lock-wait +
mysqli_poll(..., 0, 100000)timeout and validates cleared fd arrays on timeout. - Updated build/config and committed artifacts to support Asyncify instrumentation for the new/affected call paths.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/php-wasm/node/src/test/php-mysqli.spec.ts | Adds a lock-wait regression asserting mysqli_poll timeout + fd-set clearing semantics. |
| packages/php-wasm/node-builds/8-3/asyncify/php_8_3.js | Updates committed generated artifact (size + minor formatting change). |
| packages/php-wasm/compile/php/php_wasm.c | Replaces sequential per-fd waiting with shared-deadline polling and output fd-set filtering. |
| packages/php-wasm/compile/php/Dockerfile | Extends Asyncify imports to include mysqli/mysqlnd functions involved in the call path. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+950
to
+976
| int ready_count = 0; | ||
| for (int i = 0; i < poll_fd_count; i++) | ||
| { | ||
| const int fd = poll_fds[i].fd; | ||
| const short revents = poll_fds[i].revents; | ||
| if (revents & POLLNVAL) | ||
| { | ||
| free(poll_fds); | ||
| errno = EBADF; | ||
| return -1; | ||
| } | ||
|
|
||
| if (read_fds && FD_ISSET(fd, &original_read_fds) && (revents & (POLLIN | POLLERR | POLLHUP))) | ||
| { | ||
| FD_SET(fd, read_fds); | ||
| ready_count++; | ||
| } | ||
| if (write_fds && FD_ISSET(fd, &original_write_fds) && (revents & (POLLOUT | POLLERR))) | ||
| { | ||
| FD_SET(fd, write_fds); | ||
| ready_count++; | ||
| } | ||
| if (except_fds && FD_ISSET(fd, &original_except_fds) && (revents & POLLPRI)) | ||
| { | ||
| FD_SET(fd, except_fds); | ||
| ready_count++; | ||
| } |
Comment on lines
+886
to
+891
| php_pollfd *poll_fds = max_fd > 0 ? calloc(max_fd, sizeof(php_pollfd)) : NULL; | ||
| if (max_fd > 0 && !poll_fds) | ||
| { | ||
| errno = ENOMEM; | ||
| return -1; | ||
| } |
| * a valid pipe whose first bytes have not arrived yet. | ||
| */ | ||
| cp.stdin.end(); | ||
| */ cp.stdin.end(); |
chubes4
marked this pull request as draft
August 19, 2026 07:56
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
select()wrapper's sequential per-descriptor waits with a shared-deadline cooperative polling loopmysqli_pollcall pathFixes #4161.
Downstream: Automattic/wp-codebox#2010.
Root cause
PHP 8.3 follows this path:
The previous wrapper did not implement
select()semantics:POLLIN | POLLOUT, so ordinary writability could masquerade as readable MySQL dataThe new wrapper performs synchronous zero-timeout
php_poll2()checks across all unique descriptors, filters the output sets from actual readiness, and yields in bounded increments throughemscripten_sleep()until one shared monotonic deadline. This uses an existing supported suspension primitive in both JSPI and Asyncify and removeswasm_poll_socket()'s lossy boolean/sequential behavior from the multi-descriptorselect()path.Asyncify instrumentation
The wrapper fix alone is not sufficient.
ASYNCIFY_ONLYis a hand-maintained allowlist, and the async mysqli path had three separate gaps. Each was only observable once the previous one was cleared and the binary rebuilt:mysqlnd_stream_array_to_fd_set.mysqlnd_poll()walks the fd sets twice and both sides make the same suspending call —php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL, ...)atmysqlnd_connection.c:2177and:2209. Only thefrom_side was instrumented;to_runs first.tx_begin_pub. With the poll path cleared, the rebuild surfaced:tx_commit_or_rollback_pubwas listed in both name variants;tx_begin_pubin neither. An async lock-wait cannot be set up without a transaction, so the poll path was unreachable in practice regardless of the poll fix.The rest of the lifecycle. Rather than pay a rebuild per trap, every mysqli call the regression makes was enumerated and checked against the list:
zif_mysqli_close,zif_mysqli_report,zif_mysqli_rollback, andmysqlnd_conn_data_close_pub/send_close_pubin both name variants were all absent.mysqlnd_com_quit_runwas already instrumented while the two methods that reach it were not, so closing an async connection could trap even after a successful poll.zif_mysqli_commitis added alongsiderollback: sametx_commit_or_rollback_pubpath, would trap identically.Full delta:
Note for anyone auditing the list: the existing
stream_array_from_fd_setentry is theext/standard/streamsfuncs.chelper behindstream_select(), not the mysqlnd one. Different function, different file — themysqlnd_-prefixed entries are not duplicates. Its ownstream_array_to_fd_setsibling is still absent and makes the samephp_stream_castcall; likely the same latent class, not touched here.Regression
The mysqli integration test now:
MYSQLI_ASYNCquery on anothermysqli_poll(..., 0, 100000)returns0in a bounded intervalfinallyThe test uses a unique table name and closes a still-pending connection on exceptional paths.
Verification
Binaries rebuilt and the regression run against them in the same job, with a MySQL 8 service container:
Against the previous binaries the same test wedges the runtime until an outer timeout, which is #4161.
Build:
node packages/php-wasm/compile/build.js --PLATFORM=node --PHP_VERSION=8.3.32onubuntu-latest, Emscripten 4.0.19 from the pinnedbase-image/Dockerfile. ~14 minutes.Notes on the regenerated artifact
Only the Node Asyncify 8.3 target is regenerated here — the one this change affects and the one the regression runs against. Other versions and the JSPI/web targets still need the standard rebuild before merge.
Pinned to the committed patch level.
build.jsrefreshessupported-php-versionsand defaults to the newest patch, which produced 8.3.33 against a tree that commits 8.3.32. Built at 8.3.32 instead so the binary delta carries the instrumentation change and not a PHP version bump. Happy to move to 8.3.33 if preferred.Formatted before committing.
node-buildsis not in.prettierignore, so the committed glue is prettier-formatted while Emscripten's raw output is not. Committing it raw produces ~12k lines of pure reflow around a two-line change. After formatting, the only line that moves is the one that should:+6,572 bytes of wasm, which is the added instrumentation.
Draft
Left as a draft pending a maintainer decision on artifact provenance and on whether the remaining version/platform targets should be regenerated in this PR or separately.