[PHP] Call php.mount instead of php.FS.mount in proxyFileSystem#3346
Merged
[PHP] Call php.mount instead of php.FS.mount in proxyFileSystem#3346
Conversation
80f7fac to
7fd4941
Compare
When PHP code calls proc_open() with the php binary multiple times in a row, only the first call returns output. Subsequent calls fail silently. This breaks any PHP code that spawns multiple PHP subprocesses, including WP-CLI and Blueprints v2. The root cause is that main() calls exit(), which in Emscripten throws an exception that unwinds the stack before cleanup code can run. On JSPI this leaves stdout/stderr redirected, making the module unusable for another cli() call. The fix forces runtime rotation before repeated cli() calls, giving each invocation a fresh WASM module. Also fixed: - MEMFS snapshot taken before exit() so WASM memory detach can't corrupt the filesystem copy - CWD restoration after rotation falls back to / instead of throwing - request.error rotation guard scoped to HTTP-serving instances - .finally() in cli() only schedules rotation for HTTP-serving instances Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The maxRequests rotation counter was gated on #phpWasmInitCalled, but that flag isn't set until php_wasm_init() runs inside the execution function. This meant the very first run() call never incremented the counter, breaking rotation tests that expected rotation after exactly maxRequests calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On JSPI, main()→exit() throws an Emscripten exception that prevents run_cli() from restoring stdout/stderr. This means a PHP instance that has already called cli() once cannot call it again — the second call hangs because the module is in a dirty state. This is the root cause of proc_open(php) failing on the 2nd and 3rd calls: each proc_open acquires a subprocess instance from the pool, but the pool reuses instances. The second proc_open gets the same instance back, tries to call cli() again, and hangs. The fix: when #cliDirtiedRuntime is true, mark the runtime as needing rotation before the next cli() call. The #executeWithErrorHandling method then creates a fresh WASM module via rotateRuntime(), giving us a clean slate for the next main() call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the spawn handler catches an object that isn't an Error instance (e.g. an Emscripten ExitStatus or FS.ErrnoError), use JSON.stringify with Object.getOwnPropertyNames to capture all properties instead of just showing [object Object]. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Subprocess instances (proc_open) need runtime rotation on JSPI because main()→exit() throws before cleanup code runs, leaving the module in a dirty state. But rotation was destroying PROXYFS mounts (/wordpress, /tmp, etc.) because they were set up via direct FS.mount() calls that bypassed php.mount() and weren't tracked in #mounts. Changed proxyFileSystem() to register mounts through php.mount() so hotSwapPHPRuntime() can re-apply them after creating a fresh WASM module. This also re-patches PROXYFS with mmap support on each re-mount, ensuring ICU data files remain accessible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3bf276d to
3f439a1
Compare
Now that PROXYFS mounts are registered via php.mount() and survive rotation, we can simplify the rotation logic: - Remove the phpWasmInitCalled guards that prevented rotation for CLI-only instances. They were a workaround for when PROXYFS mounts weren't tracked in #mounts. - Remove the cliDirtiedRuntime flag. It was redundant with needsRotating – every write was immediately followed by setting needsRotating, and every read just checked whether to set needsRotating. - Remove dead code after main() in run_cli(). PHP CLI's main() always calls exit(), which Emscripten turns into a thrown ExitStatus exception. The stream restoration, env cleanup, and free() never executed. This applies identically on both asyncify and JSPI – the comments claiming it was JSPI-specific were inaccurate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3f439a1 to
2c9c09e
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Fixes
proc_open('php -r "..."')silently returning empty output on the 2nd and subsequent calls.Implementation
The output was empty as the rotated runtime lost access to the site's filesystem. Every non-first PHP runtime mounts the document root and some
/internaldirectories from the primary PHP's filesystem. That is done in theproxyFileSystem()call. Now, this mounting operation calledphp.FS.mount()which directly created a mounted resource in the Emscripten Filesystem. It should have usedphp.mount()instead.At the end of the CLI call, the PHP runtime is invalidated and marked for rotation. During the rotation, every mount created through
php.mount()is reassigned to the new runtime and every mount created directly viaphp.FS.mount()is lost.So this fix replaces the
php.FS.mount()call with aphp.mount()call. That one is async, so the codebase now has moreawaitstatements.Other changes
Also improved error reporting in
sandboxedSpawnHandlerFactory— non-Errorexception objects (like Emscripten abort objects) are now logged to stderr instead of being silently swallowed.Testing instructions