Skip to content

Commit 26d9d7b

Browse files
committed
Enforce single PHP instance per CLI worker for correct file locking
When PHP acquires file locks via flock(), the locks are managed at the kernel level per-process. Multiple PHP WASM instances within the same OS process would share those locks, defeating the purpose of file locking for concurrent access control. This change ensures CLI workers use exactly one PHP instance by: - Adding maxPhpInstances option to PHPRequestHandler that, when set to 1, bypasses PHPProcessManager entirely and uses a single PHP instance - Refactoring spawn handler into a unified createPhpSpawnHandler() that accepts a configurable phpCliHandler for subprocess spawning - CLI workers now use createPhpSpawnHandler without a PHP CLI handler, meaning proc_open('php ...') returns exit code 127 instead of spawning in-process - Web environments continue using sandboxedSpawnHandlerFactory which spawns PHP instances via PHPProcessManager The spawn handler API is now: - createPhpSpawnHandler({ getPrimaryPhp, phpCliHandler? }) - core handler - createProcessManagerCliHandler(acquirePHPInstance) - for multi-instance mode - sandboxedSpawnHandlerFactory(requestHandler) - legacy wrapper
1 parent 10277ed commit 26d9d7b

File tree

16 files changed

+565
-195
lines changed

16 files changed

+565
-195
lines changed

packages/php-wasm/node/src/lib/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ export * from './node-fs-mount';
66
export * from './file-lock-manager';
77
export * from './file-lock-manager-for-node';
88
export * from './xdebug/with-xdebug';
9+
export * from './node-php-cli-handler';
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import { spawn } from 'child_process';
2+
import type { PHPCliHandler, ProcessOptions } from '@php-wasm/universal';
3+
import type { ProcessApi } from '@php-wasm/util';
4+
5+
export interface NodePhpCliHandlerOptions {
6+
/**
7+
* Path to the Node.js binary.
8+
* Defaults to process.execPath.
9+
*/
10+
nodePath?: string;
11+
12+
/**
13+
* Node.js flags to pass to the Node.js binary.
14+
* Defaults to process.execArgv.
15+
*/
16+
nodeArgs?: string[];
17+
18+
/**
19+
* Path to the php-wasm CLI entry point script.
20+
* This should be the path to the compiled php-wasm CLI binary.
21+
*/
22+
phpWasmEntryPoint: string;
23+
}
24+
25+
/**
26+
* Creates a PHP CLI handler for Node.js environments that spawns
27+
* a new OS process for each PHP subprocess.
28+
*
29+
* This is essential for CLI environments where each PHP instance needs
30+
* its own OS process for correct file locking behavior. Kernel-level
31+
* file locks (flock) are per-process, so multiple PHP WASM instances
32+
* in the same OS process would share locks and defeat the purpose of locking.
33+
*
34+
* When PHP calls proc_open('php ...'), this handler spawns a completely
35+
* separate OS process running the php-wasm CLI, ensuring proper isolation.
36+
*
37+
* @example
38+
* ```ts
39+
* const phpCliHandler = createNodePhpCliHandler({
40+
* phpWasmEntryPoint: '/path/to/php-wasm-cli/main.js',
41+
* });
42+
*
43+
* const spawnHandler = createPhpSpawnHandler({
44+
* getPrimaryPhp: () => requestHandler.getPrimaryPhp(),
45+
* phpCliHandler,
46+
* });
47+
* ```
48+
*/
49+
export function createNodePhpCliHandler(
50+
options: NodePhpCliHandlerOptions
51+
): PHPCliHandler {
52+
const {
53+
nodePath = process.execPath,
54+
nodeArgs = process.execArgv,
55+
phpWasmEntryPoint,
56+
} = options;
57+
58+
return async (
59+
args: string[],
60+
processApi: ProcessApi,
61+
processOptions: ProcessOptions
62+
) => {
63+
// Skip 'php' from args since we're invoking the php-wasm CLI directly
64+
const phpArgs = args.slice(1);
65+
66+
const child = spawn(
67+
nodePath,
68+
[...nodeArgs, phpWasmEntryPoint, ...phpArgs],
69+
{
70+
cwd: processOptions.cwd,
71+
env: {
72+
...process.env,
73+
...processOptions.env,
74+
// Set SHELL_PIPE to 0 to ensure WP-CLI formats
75+
// the output as ASCII tables.
76+
// @see https://github.com/wp-cli/wp-cli/issues/1102
77+
SHELL_PIPE: '0',
78+
},
79+
stdio: ['pipe', 'pipe', 'pipe'],
80+
}
81+
);
82+
83+
// Handle stdin from the parent process
84+
processApi.on('stdin', (data: Uint8Array | string) => {
85+
if (child.stdin && !child.stdin.destroyed) {
86+
child.stdin.write(data);
87+
}
88+
});
89+
90+
// When the parent process closes stdin, close child's stdin too
91+
processApi.childProcess.stdin?.on('finish', () => {
92+
child.stdin?.end();
93+
});
94+
95+
// Stream stdout to the parent process
96+
child.stdout?.on('data', (chunk: Buffer) => {
97+
processApi.stdout(chunk);
98+
});
99+
100+
// Stream stderr to the parent process
101+
child.stderr?.on('data', (chunk: Buffer) => {
102+
processApi.stderr(chunk);
103+
});
104+
105+
// Wait for the child process to exit
106+
await new Promise<void>((resolve) => {
107+
child.on('close', (code) => {
108+
processApi.exit(code ?? 0);
109+
resolve();
110+
});
111+
112+
child.on('error', (error) => {
113+
processApi.stderr(
114+
`Failed to spawn PHP process: ${error.message}\n`
115+
);
116+
processApi.exit(1);
117+
resolve();
118+
});
119+
});
120+
};
121+
}

packages/php-wasm/node/src/test/php-part-1.spec.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {
22
getPhpIniEntries,
33
PHP,
4-
PHPProcessManager,
4+
PHPRequestHandler,
55
sandboxedSpawnHandlerFactory,
66
setPhpIniEntries,
77
type SpawnedPHP,
@@ -1830,9 +1830,9 @@ describe('sandboxedSpawnHandlerFactory', () => {
18301830
const phpVersion = RecommendedPHPVersion;
18311831
let php: PHP;
18321832
let spawnedPhp: SpawnedPHP;
1833-
let processManager: PHPProcessManager;
1833+
let requestHandler: PHPRequestHandler;
18341834
beforeEach(async () => {
1835-
processManager = new PHPProcessManager({
1835+
requestHandler = new PHPRequestHandler({
18361836
phpFactory: async () => {
18371837
const php = new PHP(
18381838
await loadNodeRuntime(phpVersion as any, {})
@@ -1850,17 +1850,19 @@ describe('sandboxedSpawnHandlerFactory', () => {
18501850
'Hello, world!'
18511851
);
18521852
await php.setSpawnHandler(
1853-
sandboxedSpawnHandlerFactory(processManager)
1853+
sandboxedSpawnHandlerFactory(requestHandler)
18541854
);
18551855
return php;
18561856
},
18571857
maxPhpInstances: 5,
1858+
documentRoot: '/tmp/shared-test-directory',
1859+
absoluteUrl: 'http://localhost',
18581860
});
1859-
spawnedPhp = await processManager.acquirePHPInstance();
1861+
spawnedPhp = await requestHandler.processManager!.acquirePHPInstance();
18601862
php = spawnedPhp.php;
18611863
});
18621864
afterEach(async () => {
1863-
await processManager[Symbol.asyncDispose]();
1865+
await requestHandler[Symbol.asyncDispose]();
18641866
spawnedPhp?.reap();
18651867
});
18661868
it.each([

packages/php-wasm/node/src/test/php-part-2.spec.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {
22
__private__dont__use,
33
loadPHPRuntime,
44
PHP,
5-
PHPProcessManager,
5+
PHPRequestHandler,
66
sandboxedSpawnHandlerFactory,
77
setPhpIniEntries,
88
type SpawnedPHP,
@@ -1160,9 +1160,9 @@ describe('sandboxedSpawnHandlerFactory', () => {
11601160
const phpVersion = RecommendedPHPVersion;
11611161
let php: PHP;
11621162
let spawnedPhp: SpawnedPHP;
1163-
let processManager: PHPProcessManager;
1163+
let requestHandler: PHPRequestHandler;
11641164
beforeEach(async () => {
1165-
processManager = new PHPProcessManager({
1165+
requestHandler = new PHPRequestHandler({
11661166
phpFactory: async () => {
11671167
const php = new PHP(
11681168
await loadNodeRuntime(phpVersion as any, {})
@@ -1180,17 +1180,19 @@ describe('sandboxedSpawnHandlerFactory', () => {
11801180
'Hello, world!'
11811181
);
11821182
await php.setSpawnHandler(
1183-
sandboxedSpawnHandlerFactory(processManager)
1183+
sandboxedSpawnHandlerFactory(requestHandler)
11841184
);
11851185
return php;
11861186
},
11871187
maxPhpInstances: 5,
1188+
documentRoot: '/tmp/shared-test-directory',
1189+
absoluteUrl: 'http://localhost',
11881190
});
1189-
spawnedPhp = await processManager.acquirePHPInstance();
1191+
spawnedPhp = await requestHandler.processManager!.acquirePHPInstance();
11901192
php = spawnedPhp.php;
11911193
});
11921194
afterEach(async () => {
1193-
await processManager[Symbol.asyncDispose]();
1195+
await requestHandler[Symbol.asyncDispose]();
11941196
spawnedPhp?.reap();
11951197
});
11961198
it.each([

packages/php-wasm/universal/src/lib/index.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,15 @@ export { HttpCookieStore } from './http-cookie-store';
2424
export type { IteratePhpFilesOptions as IterateFilesOptions } from './iterate-files';
2525
export { iteratePhpFiles as iterateFiles } from './iterate-files';
2626
export { writeFilesStreamToPhp } from './write-files-stream-to-php';
27-
export { PHPProcessManager } from './php-process-manager';
27+
export {
28+
PHPProcessManager,
29+
SingularPHPProcessManager,
30+
} from './php-process-manager';
2831
export type {
2932
MaxPhpInstancesError,
3033
PHPFactory,
3134
PHPFactoryOptions,
35+
PHPProcessManagerInterface,
3236
ProcessManagerOptions,
3337
SpawnedPHP,
3438
} from './php-process-manager';
@@ -84,7 +88,14 @@ export {
8488

8589
export { isExitCode } from './is-exit-code';
8690
export { proxyFileSystem, isPathToSharedFS } from './proxy-file-system';
87-
export { sandboxedSpawnHandlerFactory } from './sandboxed-spawn-handler-factory';
91+
export {
92+
createPhpSpawnHandler,
93+
createProcessManagerCliHandler,
94+
sandboxedSpawnHandlerFactory,
95+
type PHPCliHandler,
96+
type ProcessOptions,
97+
type SpawnHandlerOptions,
98+
} from './sandboxed-spawn-handler-factory';
8899

89100
export * from './api';
90101
export type { WithAPIState as WithIsReady } from './api';

packages/php-wasm/universal/src/lib/php-process-manager.ts

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,16 @@ export type PHPFactoryOptions = {
77

88
export type PHPFactory = (options: PHPFactoryOptions) => Promise<PHP>;
99

10+
/**
11+
* Common interface for PHP process managers.
12+
*/
13+
export interface PHPProcessManagerInterface extends AsyncDisposable {
14+
getPrimaryPhp(): Promise<PHP>;
15+
acquirePHPInstance(options?: {
16+
considerPrimary?: boolean;
17+
}): Promise<SpawnedPHP>;
18+
}
19+
1020
export interface ProcessManagerOptions {
1121
/**
1222
* The maximum number of PHP instances that can exist at
@@ -71,7 +81,7 @@ export class MaxPhpInstancesError extends Error {
7181
* requests requires extra time to spin up a few PHP instances. This is a more
7282
* resource-friendly tradeoff than keeping 5 idle instances at all times.
7383
*/
74-
export class PHPProcessManager implements AsyncDisposable {
84+
export class PHPProcessManager implements PHPProcessManagerInterface {
7585
private primaryPhp?: PHP;
7686
private primaryPhpPromise?: Promise<SpawnedPHP>;
7787
private primaryIdle = true;
@@ -266,3 +276,66 @@ export class PHPProcessManager implements AsyncDisposable {
266276
);
267277
}
268278
}
279+
280+
/**
281+
* A PHP Process manager that manages exactly one PHP instance.
282+
*
283+
* This is useful for CLI environments where we want each OS process to have
284+
* exactly one PHP instance for correct file locking behavior. Kernel-level
285+
* file locks are per-process, so multiple PHP instances in the same process
286+
* would share locks and defeat the purpose of locking.
287+
*
288+
* Unlike PHPProcessManager which can spawn multiple instances, this manager:
289+
* - Never creates or destroys PHP instances (the instance is provided at construction)
290+
* - Refuses to acquire if the instance is already acquired
291+
* - Implements the same interface as PHPProcessManager for compatibility
292+
*/
293+
export class SingularPHPProcessManager implements PHPProcessManagerInterface {
294+
private php: PHP;
295+
private phpPromise?: Promise<PHP>;
296+
private phpFactory?: PHPFactory;
297+
private acquired = false;
298+
299+
constructor(options: { php?: PHP; phpFactory?: PHPFactory }) {
300+
if (!options.php && !options.phpFactory) {
301+
throw new Error(
302+
'SingularPHPProcessManager requires either php or phpFactory'
303+
);
304+
}
305+
this.php = options.php!;
306+
this.phpFactory = options.phpFactory;
307+
}
308+
309+
async getPrimaryPhp(): Promise<PHP> {
310+
if (!this.php) {
311+
if (!this.phpPromise) {
312+
this.phpPromise = this.phpFactory!({ isPrimary: true });
313+
}
314+
this.php = await this.phpPromise;
315+
this.phpPromise = undefined;
316+
}
317+
return this.php;
318+
}
319+
320+
async acquirePHPInstance(): Promise<SpawnedPHP> {
321+
if (this.acquired) {
322+
throw new AcquireTimeoutError(
323+
'SingularPHPProcessManager: Cannot acquire PHP instance - already in use. ' +
324+
'This manager only supports one concurrent acquisition.'
325+
);
326+
}
327+
this.acquired = true;
328+
return {
329+
php: await this.getPrimaryPhp(),
330+
reap: () => {
331+
this.acquired = false;
332+
},
333+
};
334+
}
335+
336+
async [Symbol.asyncDispose]() {
337+
if (this.php) {
338+
this.php.exit();
339+
}
340+
}
341+
}

0 commit comments

Comments
 (0)