[CLI] Explore spawning new OS processes in CLI spawnProcess #3001
Conversation
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
adamziel
force-pushed
the
do-max-1-php-instance-per-cli-worker
branch
from
af6e674 to
ad31298
Compare
adamziel
added
[Type] Enhancement
adamziel
added a commit
that referenced
this pull request
Dec 9, 2025
When PHP calls proc_open() to spawn a subprocess, each subprocess now runs in a separate Node.js worker thread. This ensures OS-level file locks via fcntl() and LockFileEx() properly conflict between parent and child processes, matching native PHP behavior. This is the V2 counterpart to the V1 worker change in PR #3001.
brandonpayton
approved these changes
Dec 9, 2025
Member
brandonpayton
left a comment
brandonpayton
left a comment
There was a problem hiding this comment.
These changes read well to me, and the tests are passing.
There is a random hello() worker method that may be leftover from dev, but it can be easily removed later. So I'll go ahead and merge this.
Comment on lines
+17
to
+20
| getPHPInstance: () => Promise<{ | ||
| php: PHP | Remote<PHPWorker>; | ||
| reap: () => void; | ||
| }> |
Comment on lines
+212
to
+215
| async hello() { | ||
| return 'hello'; | ||
| } | ||
|
|
Member
There was a problem hiding this comment.
Hello! Is this left over from a development test, and do we still want it?
Collaborator
Author
There was a problem hiding this comment.
yeah it's a testing leftover, good spot
| } | ||
| ) | ||
| ); | ||
| const worker = spawnWorkerThread(workerType, { onExit }); |
Member
There was a problem hiding this comment.
Nice change -- moving the new Promise() logic into spawnWorkerThread().
Collaborator
Author
There was a problem hiding this comment.
Yes, this enables reusing it :)
adamziel
added a commit
that referenced
this pull request
Dec 10, 2025
When PHP calls proc_open() to spawn a subprocess, each subprocess now runs in a separate Node.js worker thread. This ensures OS-level file locks via fcntl() and LockFileEx() properly conflict between parent and child processes, matching native PHP behavior. This is the V2 counterpart to the V1 worker change in PR #3001.
adamziel
added a commit
that referenced
this pull request
Dec 10, 2025
…ndler in Blueprint V2 worker (#3004) ## Motivation Makes the Blueprints V2 worker spawn a new OS process when PHP calls `proc_open()`. This enables using OS-level `flock()`, `LockFileEx()`, etc. Before this PR, `proc_open()` would create a new PHP instance within the same Node.js worker, which meant all the OS-level file locks acquired by that "PHP sub-process" would be attributed to the same OS-level process as the "parent PHP process". Follows up on the V1 worker PR #3001. This PR also ships the changes from #3006: Introduces `SinglePHPInstanceManager`, a `PHPProcessManager` alternative to use with `PHPRequestHandler` that uses a single PHP instance without spawning another one or killing it. Note the internal runtime rotation after n requests is still supported separately via `php.enableRuntimeRotation()`. `PHPProcessManager` was designed to use multiple PHP instances. However, in CLI, we can only use a single PHP instance per process if we want to support exclusive file locking via `flock()` and `LockFileEx()`. ## Implementation ### Spawn The `bootRequestHandler` method now passes a custom PHP-spawning factory function to `sandboxedSpawnHandlerFactory`. ### Request handler Introduce a `PHPInstanceManager` interface that abstracts PHP instance lifecycle management: ```typescript interface PHPInstanceManager extends AsyncDisposable { getPrimaryPhp(): Promise<PHP>; acquirePHPInstance(options?: { considerPrimary?: boolean }): Promise<AcquiredPHP>; } ``` Two implementations: - **PHPProcessManager**: Multiple instances with semaphore-based concurrency control (web) - **SinglePHPInstanceManager**: Single instance, no concurrency overhead (CLI) PHPRequestHandler now accepts either configuration: ```typescript // Option 1: Provide an instance manager directly const requestHandler = new PHPRequestHandler({ instanceManager: new SinglePHPInstanceManager({ phpFactory: ... }), documentRoot: '/wordpress', }); // Option 2: Provide a factory (creates PHPProcessManager internally, unchanged from before) const requestHandler = new PHPRequestHandler({ phpFactory: async () => loadWebRuntime('8.0'), maxPhpInstances: 5, }); ``` ## Testing CI
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.
Motivation
This PR changes the Playground CLI so that every PHP subprocess spawned by another PHP instance (e.g.
proc_open('php -v')) is started in a separate OS process.When the sandboxed spawn handler detects a PHP subprocess request, it now creates an entirely new Node.js worker thread running. Any file locks acquired by the subprocess will properly conflict with the parent's locks.
Before this PR two PHP instances running in the same OS process could acquire overlapping OS-level locks (via
fcntl()andLockFileEx()).Implementation
The way I'm augmenting
sandboxedSpawnHandlerFactorycalls in this PR is somewhat convoluted. I'd like to revisit it once we finish the OS-locks-related refactoring of the CLI code.The
sandboxedSpawnHandlerFactorynow accepts a callback that creates a new PHP worker. When a PHP subprocess is requested viaproc_open(), the CLI:spawnWorkerThread('v1')bootWorker()The key change is in
worker-thread-v1.tswherecreatePHPWorker()now spawns an entirely new OS process instead of reusing the existing PHP process manager's pool.Follow-up work
@php-wasm/cliPHPProcessManagerusage in CLI workers