Skip to content

Conversation

@adamziel
Copy link
Collaborator

@adamziel adamziel commented Dec 9, 2025

Motivation

This is the V2 counterpart to PR #3001. When PHP calls proc_open() to spawn a subprocess (like WP-CLI calling PHP), we need to ensure those subprocesses don't interfere with the parent's file locks.

Previously in the V2 worker, sandboxedSpawnHandlerFactory was passed directly without a callback to create new OS processes. Subprocesses ran as new PHP instances within the same Node.js worker, which meant OS-level file locks (via fcntl() and LockFileEx()) could overlap between parent and child.

This PR applies the same pattern from the V1 worker: spawning each subprocess in a separate Node.js worker thread running worker-thread-v2.ts.

Implementation

The bootRequestHandler method now wraps sandboxedSpawnHandlerFactory with a callback that:

  1. Spawns a new Node.js worker thread via spawnWorkerThread('v2')
  2. Boots a fresh PHP runtime in that worker using bootWorker()
  3. Connects the subprocess I/O streams to the parent
  4. Terminates the worker when the subprocess exits

Testing

This change affects the experimental Blueprint V2 runner. Run WP-CLI commands that spawn subprocesses:

npx wp-playground-cli server --experimental-blueprints-v2-runner --mount ./my-plugin:/wordpress/wp-content/plugins/my-plugin
# In another terminal:
curl "http://127.0.0.1:9400/?cli=wp+plugin+list"

PHPRequestHandler now always creates its own PHPProcessManager internally.
The configuration is simplified to always require a phpFactory callback,
removing the union type that allowed passing either a processManager or
a phpFactory.

This reduces API surface and complexity since no callers were using
the custom processManager option.
Introduces a PHPInstanceManager interface that abstracts PHP instance
lifecycle management. Both PHPProcessManager (for web contexts with
multiple concurrent instances) and the new SinglePHPInstanceManager
(for CLI contexts with a single instance) implement this interface.

PHPRequestHandler now accepts either:
- instanceManager: A pre-built PHPInstanceManager instance
- phpFactory + maxPhpInstances: Creates PHPProcessManager internally

This allows CLI contexts to use a simpler single-instance manager where
runtime rotation is handled separately via php.enableRuntimeRotation(),
while web contexts continue using PHPProcessManager for concurrency.
The spawnHandler callback parameter is now typed as PHPInstanceManager
instead of PHPProcessManager to align with the interface abstraction.
PHPRequestHandler now supports three configuration options:
- instanceManager: Custom PHPInstanceManager (advanced)
- php: Single PHP instance (creates SinglePHPInstanceManager internally)
- phpFactory + maxPhpInstances: Factory function (creates PHPProcessManager)

When a php instance is provided directly, the same transformations
(mkdir documentRoot, chdir, set requestHandler) are applied as with
phpFactory, keeping the behavior consistent.
Simplify PHPRequestHandler configuration to only two options:
- php: Single PHP instance (creates SinglePHPInstanceManager internally)
- phpFactory + maxPhpInstances: Factory function (creates PHPProcessManager)

The instanceManager option was overly complex for actual use cases.
The AcquiredPHP type better describes what the interface represents:
a PHP instance that has been acquired from the instance manager and
must be reaped when done. SpawnedPHP is now a backwards-compatible
type alias exported from index.ts.
Base automatically changed from do-max-1-php-instance-per-cli-worker to trunk December 9, 2025 21:57
The sandboxedSpawnHandlerFactory now expects a getPHPInstance callback.
Update boot.ts to use instanceManager instead of processManager and add
a null check for requestHandler.
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.
Pass the options object directly instead of destructuring and
reconstructing it.
@adamziel adamziel force-pushed the do-max-1-php-instance-per-cli-worker-v2 branch from 69e8dd5 to a3aaff1 Compare December 10, 2025 01:01
@adamziel adamziel requested a review from a team as a code owner December 10, 2025 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants