Skip to content

Commit c912ca0

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 c912ca0

File tree

14 files changed

+407
-193
lines changed

14 files changed

+407
-193
lines changed

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: 12 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,13 @@ 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 SpawnHandlerOptions,
97+
} from './sandboxed-spawn-handler-factory';
8898

8999
export * from './api';
90100
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+
}

packages/php-wasm/universal/src/lib/php-request-handler.ts

Lines changed: 44 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,16 @@ import { normalizeHeaders } from './php';
1010
import { PHPResponse } from './php-response';
1111
import type { PHPRequest, PHPRunOptions } from './universal-php';
1212
import { encodeAsMultipart } from './encode-as-multipart';
13-
import type { PHPFactoryOptions, SpawnedPHP } from './php-process-manager';
14-
import { MaxPhpInstancesError, PHPProcessManager } from './php-process-manager';
13+
import type {
14+
PHPFactoryOptions,
15+
PHPProcessManagerInterface,
16+
SpawnedPHP,
17+
} from './php-process-manager';
18+
import {
19+
MaxPhpInstancesError,
20+
PHPProcessManager,
21+
SingularPHPProcessManager,
22+
} from './php-process-manager';
1523
import { HttpCookieStore } from './http-cookie-store';
1624
import mimeTypes from './mime-types.json';
1725

@@ -84,35 +92,16 @@ export type PHPRequestHandlerFactoryArgs = PHPFactoryOptions & {
8492
requestHandler: PHPRequestHandler;
8593
};
8694

87-
export type PHPRequestHandlerConfiguration = BaseConfiguration &
88-
(
89-
| {
90-
/**
91-
* PHPProcessManager is required because the request handler needs
92-
* to make a decision for each request.
93-
*
94-
* Static assets are served using the primary PHP's filesystem, even
95-
* when serving 100 static files concurrently. No new PHP interpreter
96-
* is ever created as there's no need for it.
97-
*
98-
* Dynamic PHP requests, however, require grabbing an available PHP
99-
* interpreter, and that's where the PHPProcessManager comes in.
100-
*/
101-
processManager: PHPProcessManager;
102-
}
103-
| {
104-
phpFactory: (
105-
requestHandler: PHPRequestHandlerFactoryArgs
106-
) => Promise<PHP>;
107-
/**
108-
* The maximum number of PHP instances that can exist at
109-
* the same time.
110-
*/
111-
maxPhpInstances?: number;
112-
}
113-
) & {
114-
cookieStore?: CookieStore | false;
115-
};
95+
export type PHPRequestHandlerConfiguration = BaseConfiguration & {
96+
phpFactory: (requestHandler: PHPRequestHandlerFactoryArgs) => Promise<PHP>;
97+
/**
98+
* The maximum number of PHP instances that can exist at
99+
* the same time.
100+
*/
101+
maxPhpInstances?: number;
102+
} & {
103+
cookieStore?: CookieStore | false;
104+
};
116105

117106
/**
118107
* Handles HTTP requests using PHP runtime as a backend.
@@ -178,7 +167,7 @@ export class PHPRequestHandler implements AsyncDisposable {
178167
#ABSOLUTE_URL: string;
179168
#cookieStore: CookieStore | false;
180169
rewriteRules: RewriteRule[];
181-
processManager: PHPProcessManager;
170+
processManager: PHPProcessManagerInterface;
182171
getFileNotFoundAction: FileNotFoundGetActionCallback;
183172

184173
/**
@@ -202,26 +191,28 @@ export class PHPRequestHandler implements AsyncDisposable {
202191
getFileNotFoundAction = () => ({ type: '404' }),
203192
} = config;
204193

205-
if ('processManager' in config) {
206-
this.processManager = config.processManager;
194+
const phpFactory = async (info: PHPFactoryOptions) => {
195+
const php = await config.phpFactory!({
196+
...info,
197+
requestHandler: this,
198+
});
199+
200+
// Always set managed PHP's cwd to the document root.
201+
if (!php.isDir(documentRoot)) {
202+
php.mkdir(documentRoot);
203+
}
204+
php.chdir(documentRoot);
205+
206+
// @TODO: Decouple PHP and request handler
207+
(php as any).requestHandler = this;
208+
return php;
209+
};
210+
211+
if (config.maxPhpInstances === 1) {
212+
this.processManager = new SingularPHPProcessManager({ phpFactory });
207213
} else {
208214
this.processManager = new PHPProcessManager({
209-
phpFactory: async (info) => {
210-
const php = await config.phpFactory!({
211-
...info,
212-
requestHandler: this,
213-
});
214-
215-
// Always set managed PHP's cwd to the document root.
216-
if (!php.isDir(documentRoot)) {
217-
php.mkdir(documentRoot);
218-
}
219-
php.chdir(documentRoot);
220-
221-
// @TODO: Decouple PHP and request handler
222-
(php as any).requestHandler = this;
223-
return php;
224-
},
215+
phpFactory,
225216
maxPhpInstances: config.maxPhpInstances,
226217
});
227218
}
@@ -245,8 +236,8 @@ export class PHPRequestHandler implements AsyncDisposable {
245236
this.#PORT = url.port
246237
? Number(url.port)
247238
: url.protocol === 'https:'
248-
? 443
249-
: 80;
239+
? 443
240+
: 80;
250241
this.#PROTOCOL = (url.protocol || '').replace(':', '');
251242
const isNonStandardPort = this.#PORT !== 443 && this.#PORT !== 80;
252243
this.#HOST = [
@@ -578,7 +569,7 @@ export class PHPRequestHandler implements AsyncDisposable {
578569
): Promise<PHPResponse> {
579570
let spawnedPHP: SpawnedPHP | undefined = undefined;
580571
try {
581-
spawnedPHP = await this.processManager!.acquirePHPInstance({
572+
spawnedPHP = await this.processManager.acquirePHPInstance({
582573
considerPrimary: true,
583574
});
584575
} catch (e) {

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,10 @@ export class PHPWorker implements LimitedPHPApi, AsyncDisposable {
229229
* @returns A PHP instance with a consistent chroot.
230230
*/
231231
private async acquirePHPInstance() {
232-
const { php, reap } = await _private
233-
.get(this)!
234-
.requestHandler!.processManager.acquirePHPInstance();
232+
const requestHandler = _private.get(this)!.requestHandler!;
233+
const { php, reap } =
234+
await requestHandler.processManager.acquirePHPInstance();
235+
235236
if (this.chroot !== null) {
236237
php.chdir(this.chroot);
237238
}

0 commit comments

Comments
 (0)