Skip to content

Commit 947ed76

Browse files
authored
[CLI] Explore spawning new OS processes in CLI spawnProcess (#3001)
## 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()` and `LockFileEx()`). ## Implementation The way I'm augmenting `sandboxedSpawnHandlerFactory` calls 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 `sandboxedSpawnHandlerFactory` now accepts a callback that creates a new PHP worker. When a PHP subprocess is requested via `proc_open()`, the CLI: 1. Spawns a new Node.js worker thread via `spawnWorkerThread('v1')` 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 The key change is in `worker-thread-v1.ts` where `createPHPWorker()` now spawns an entirely new OS process instead of reusing the existing PHP process manager's pool. ## Follow-up work * Refactor PlaygroundCliBlueprintV2Worker * Refactor `@php-wasm/cli` * Limit PHPProcessManager to one PHP instance per worker even for request handling * Get rid of `PHPProcessManager` usage in CLI workers
1 parent 2f70805 commit 947ed76

File tree

7 files changed

+196
-126
lines changed

7 files changed

+196
-126
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1850,7 +1850,11 @@ describe('sandboxedSpawnHandlerFactory', () => {
18501850
'Hello, world!'
18511851
);
18521852
await php.setSpawnHandler(
1853-
sandboxedSpawnHandlerFactory(processManager)
1853+
sandboxedSpawnHandlerFactory(() =>
1854+
processManager.acquirePHPInstance({
1855+
considerPrimary: false,
1856+
})
1857+
)
18541858
);
18551859
return php;
18561860
},

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1180,7 +1180,11 @@ describe('sandboxedSpawnHandlerFactory', () => {
11801180
'Hello, world!'
11811181
);
11821182
await php.setSpawnHandler(
1183-
sandboxedSpawnHandlerFactory(processManager)
1183+
sandboxedSpawnHandlerFactory(() =>
1184+
processManager.acquirePHPInstance({
1185+
considerPrimary: false,
1186+
})
1187+
)
11841188
);
11851189
return php;
11861190
},

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,4 @@ export { sandboxedSpawnHandlerFactory } from './sandboxed-spawn-handler-factory'
8888

8989
export * from './api';
9090
export type { WithAPIState as WithIsReady } from './api';
91+
export type { Remote } from './comlink-sync';

packages/php-wasm/universal/src/lib/sandboxed-spawn-handler-factory.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { createSpawnHandler } from '@php-wasm/util';
2-
import type { PHPProcessManager } from './php-process-manager';
2+
import type { PHP } from './php';
3+
import type { PHPWorker } from './php-worker';
4+
import type { Remote } from './comlink-sync';
35

46
/**
57
* An isomorphic proc_open() handler that implements typical shell in TypeScript
@@ -12,7 +14,10 @@ import type { PHPProcessManager } from './php-process-manager';
1214
* parser.
1315
*/
1416
export function sandboxedSpawnHandlerFactory(
15-
processManager: PHPProcessManager
17+
getPHPInstance: () => Promise<{
18+
php: PHP | Remote<PHPWorker>;
19+
reap: () => void;
20+
}>
1621
) {
1722
return createSpawnHandler(async function (args, processApi, options) {
1823
processApi.notifySpawn();
@@ -63,16 +68,14 @@ export function sandboxedSpawnHandlerFactory(
6368
return;
6469
}
6570

66-
const { php, reap } = await processManager.acquirePHPInstance({
67-
considerPrimary: false,
68-
});
71+
const { php, reap } = await getPHPInstance();
6972

7073
try {
7174
if (options.cwd) {
72-
php.chdir(options.cwd as string);
75+
await php.chdir(options.cwd as string);
7376
}
7477

75-
const cwd = php.cwd();
78+
const cwd = await php.cwd();
7679
switch (binaryName) {
7780
case 'php': {
7881
// Figure out more about setting env, putenv(), etc.
@@ -105,7 +108,7 @@ export function sandboxedSpawnHandlerFactory(
105108
break;
106109
}
107110
case 'ls': {
108-
const files = php.listFiles(args[1] ?? cwd);
111+
const files = await php.listFiles(args[1] ?? cwd);
109112
for (const file of files) {
110113
processApi.stdout(file + '\n');
111114
}

packages/playground/cli/src/blueprints-v1/worker-thread-v1.ts

Lines changed: 122 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import { jspi } from 'wasm-feature-detect';
2121
import { MessageChannel, type MessagePort, parentPort } from 'worker_threads';
2222
import { mountResources } from '../mounts';
2323
import { logger } from '@php-wasm/logger';
24+
import { spawnWorkerThread } from '../run-cli';
25+
2426
import type { Mount } from '@php-wasm/cli-util';
2527

2628
export type WorkerBootOptions = {
@@ -122,31 +124,22 @@ export class PlaygroundCliBlueprintV1Worker extends PHPWorker {
122124
}
123125
}
124126

125-
async bootAndSetUpInitialWorker({
126-
siteUrl,
127-
mountsBeforeWpInstall,
128-
mountsAfterWpInstall,
129-
phpVersion: php = RecommendedPHPVersion,
130-
wordpressInstallMode,
131-
wordPressZip,
132-
sqliteIntegrationPluginZip,
133-
firstProcessId,
134-
processIdSpaceLength,
135-
dataSqlPath,
136-
followSymlinks,
137-
trace,
138-
internalCookieStore,
139-
withXdebug,
140-
nativeInternalDirPath,
141-
}: PrimaryWorkerBootOptions) {
127+
async bootAndSetUpInitialWorker(options: PrimaryWorkerBootOptions) {
128+
const {
129+
siteUrl,
130+
mountsBeforeWpInstall,
131+
mountsAfterWpInstall,
132+
wordpressInstallMode,
133+
wordPressZip,
134+
sqliteIntegrationPluginZip,
135+
dataSqlPath,
136+
internalCookieStore,
137+
} = options;
142138
if (this.booted) {
143139
throw new Error('Playground already booted');
144140
}
145141
this.booted = true;
146142

147-
let nextProcessId = firstProcessId;
148-
const lastProcessId = firstProcessId + processIdSpaceLength - 1;
149-
150143
try {
151144
const constants: Record<string, string | number | boolean | null> =
152145
{
@@ -157,27 +150,10 @@ export class PlaygroundCliBlueprintV1Worker extends PHPWorker {
157150
let wordpressBooted = false;
158151
const requestHandler = await bootWordPressAndRequestHandler({
159152
siteUrl,
160-
createPhpRuntime: async () => {
161-
const processId = nextProcessId;
162-
163-
if (nextProcessId < lastProcessId) {
164-
nextProcessId++;
165-
} else {
166-
// We've reached the end of the process ID space. Start over.
167-
nextProcessId = firstProcessId;
168-
}
169-
170-
return await loadNodeRuntime(php, {
171-
emscriptenOptions: {
172-
fileLockManager: this.fileLockManager!,
173-
processId,
174-
trace: trace ? tracePhpWasm : undefined,
175-
phpWasmInitOptions: { nativeInternalDirPath },
176-
},
177-
followSymlinks,
178-
withXdebug,
179-
});
180-
},
153+
createPhpRuntime: createPhpRuntimeFactory(
154+
options,
155+
this.fileLockManager!
156+
),
181157
wordpressInstallMode,
182158
wordPressZip:
183159
wordPressZip !== undefined
@@ -203,7 +179,10 @@ export class PlaygroundCliBlueprintV1Worker extends PHPWorker {
203179
},
204180
cookieStore: internalCookieStore ? undefined : false,
205181
dataSqlPath,
206-
spawnHandler: sandboxedSpawnHandlerFactory,
182+
spawnHandler: () =>
183+
sandboxedSpawnHandlerFactory(() =>
184+
createPHPWorker(options, this.fileLockManager!)
185+
),
207186
async onPHPInstanceCreated(php) {
208187
await mountResources(php, mountsBeforeWpInstall);
209188
if (wordpressBooted) {
@@ -230,64 +209,37 @@ export class PlaygroundCliBlueprintV1Worker extends PHPWorker {
230209
}
231210
}
232211

212+
async hello() {
213+
return 'hello';
214+
}
215+
233216
async bootWorker(args: WorkerBootOptions) {
234217
await this.bootRequestHandler(args);
235218
}
236219

237-
async bootRequestHandler({
238-
siteUrl,
239-
followSymlinks,
240-
phpVersion,
241-
firstProcessId,
242-
processIdSpaceLength,
243-
trace,
244-
nativeInternalDirPath,
245-
mountsBeforeWpInstall,
246-
mountsAfterWpInstall,
247-
withXdebug,
248-
}: WorkerBootRequestHandlerOptions) {
220+
async bootRequestHandler(options: WorkerBootRequestHandlerOptions) {
249221
if (this.booted) {
250222
throw new Error('Playground already booted');
251223
}
252224
this.booted = true;
253225

254-
let nextProcessId = firstProcessId;
255-
const lastProcessId = firstProcessId + processIdSpaceLength - 1;
256-
257226
try {
258227
const requestHandler = await bootRequestHandler({
259-
siteUrl,
260-
createPhpRuntime: async () => {
261-
const processId = nextProcessId;
262-
263-
if (nextProcessId < lastProcessId) {
264-
nextProcessId++;
265-
} else {
266-
// We've reached the end of the process ID space. Start over.
267-
nextProcessId = firstProcessId;
268-
}
269-
270-
return await loadNodeRuntime(phpVersion, {
271-
emscriptenOptions: {
272-
fileLockManager: this.fileLockManager!,
273-
processId,
274-
trace: trace ? tracePhpWasm : undefined,
275-
ENV: {
276-
DOCROOT: '/wordpress',
277-
},
278-
phpWasmInitOptions: { nativeInternalDirPath },
279-
},
280-
followSymlinks,
281-
withXdebug,
282-
});
283-
},
228+
siteUrl: options.siteUrl,
229+
createPhpRuntime: createPhpRuntimeFactory(
230+
options,
231+
this.fileLockManager!
232+
),
284233
onPHPInstanceCreated: async (php) => {
285-
await mountResources(php, mountsBeforeWpInstall);
286-
await mountResources(php, mountsAfterWpInstall);
234+
await mountResources(php, options.mountsBeforeWpInstall);
235+
await mountResources(php, options.mountsAfterWpInstall);
287236
},
288237
sapiName: 'cli',
289238
cookieStore: false,
290-
spawnHandler: sandboxedSpawnHandlerFactory,
239+
spawnHandler: () =>
240+
sandboxedSpawnHandlerFactory(() =>
241+
createPHPWorker(options, this.fileLockManager!)
242+
),
291243
});
292244
this.__internal_setRequestHandler(requestHandler);
293245

@@ -307,6 +259,91 @@ export class PlaygroundCliBlueprintV1Worker extends PHPWorker {
307259
}
308260
}
309261

262+
/**
263+
* Returns a factory function that starts a new PHP runtime in the currently
264+
* running process. This is used for rotating the PHP runtime periodically.
265+
*/
266+
function createPhpRuntimeFactory(
267+
options: WorkerBootRequestHandlerOptions,
268+
fileLockManager: FileLockManager | RemoteAPI<FileLockManager>
269+
) {
270+
let nextProcessId = options.firstProcessId;
271+
const lastProcessId =
272+
options.firstProcessId + options.processIdSpaceLength - 1;
273+
return async () => {
274+
const processId = nextProcessId;
275+
276+
if (nextProcessId < lastProcessId) {
277+
nextProcessId++;
278+
} else {
279+
// We've reached the end of the process ID space. Start over.
280+
nextProcessId = options.firstProcessId;
281+
}
282+
283+
return await loadNodeRuntime(
284+
options.phpVersion || RecommendedPHPVersion,
285+
{
286+
emscriptenOptions: {
287+
fileLockManager,
288+
processId,
289+
trace: options.trace ? tracePhpWasm : undefined,
290+
phpWasmInitOptions: {
291+
nativeInternalDirPath: options.nativeInternalDirPath,
292+
},
293+
},
294+
followSymlinks: options.followSymlinks,
295+
withXdebug: options.withXdebug,
296+
}
297+
);
298+
};
299+
}
300+
301+
/**
302+
* Spawns a new PHP process to be used in the PHP spawn handler (in proc_open() etc. calls).
303+
* It boots from this worker-thread-v1.ts file, but is a separate process.
304+
*
305+
* We explicitly avoid using PHPProcessManager.acquirePHPInstance() here.
306+
*
307+
* Why?
308+
*
309+
* Because each PHP instance acquires actual OS-level file locks via fcntl() and LockFileEx()
310+
* syscalls. Running multiple PHP instances from the same OS process would allow them to
311+
* acquire overlapping locks. Running every PHP instance in a separate OS process ensures
312+
* any locks that overlap between PHP instances conflict with each other as expected.
313+
*
314+
* @param options - The options for the worker.
315+
* @param fileLockManager - The file lock manager to use.
316+
* @returns A promise that resolves to the PHP worker.
317+
*/
318+
async function createPHPWorker(
319+
options: WorkerBootRequestHandlerOptions,
320+
fileLockManager: FileLockManager | RemoteAPI<FileLockManager>
321+
) {
322+
const spawnedWorker = await spawnWorkerThread('v1');
323+
324+
const handler = consumeAPI<PlaygroundCliBlueprintV1Worker>(
325+
spawnedWorker.phpPort
326+
);
327+
handler.useFileLockManager(fileLockManager as any);
328+
await handler.bootWorker(options);
329+
330+
return {
331+
php: handler,
332+
reap: () => {
333+
try {
334+
handler.dispose();
335+
} catch {
336+
/** */
337+
}
338+
try {
339+
spawnedWorker.worker.terminate();
340+
} catch {
341+
/** */
342+
}
343+
},
344+
};
345+
}
346+
310347
process.on('unhandledRejection', (e: any) => {
311348
logger.error('Unhandled rejection:', e);
312349
});

0 commit comments

Comments
 (0)