Skip to content

Commit 3903795

Browse files
Utsav-LadaniUtsav-Ladani
authored
Merge branch 'trunk' into add/rm-command-support
2 parents 3d7f99b + d07a2ef commit 3903795

18 files changed

+753
-176
lines changed

‎packages/php-wasm/cli/src/main.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ ${process.argv[0]} ${process.execArgv.join(' ')} ${process.argv[1]}
129129
withXdebug: hasXdebugOption,
130130
})
131131
);
132+
php.setSpawnHandler(require('child_process').spawn);
132133

133134
useHostFilesystem(php);
134135

‎packages/php-wasm/node/src/test/php-instance-manager.spec.ts‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
import { RecommendedPHPVersion } from '@wp-playground/common';
2+
import { loadNodeRuntime } from '..';
3+
import { PHP, PHPProcessManager, SinglePHPInstanceManager } from '@php-wasm/universal';
4+
5+
describe('SinglePHPInstanceManager', () => {
6+
it('should return the PHP instance passed in the constructor', async () => {
7+
const php = new PHP(await loadNodeRuntime(RecommendedPHPVersion));
8+
const mgr = new SinglePHPInstanceManager({ php });
9+
10+
const primaryPhp = await mgr.getPrimaryPhp();
11+
expect(primaryPhp).toBe(php);
12+
});
13+
14+
it('should create a PHP instance using the factory when no instance is provided', async () => {
15+
const phpFactory = vitest.fn(
16+
async () => new PHP(await loadNodeRuntime(RecommendedPHPVersion))
17+
);
18+
const mgr = new SinglePHPInstanceManager({ phpFactory });
19+
20+
expect(phpFactory).not.toHaveBeenCalled();
21+
const php = await mgr.getPrimaryPhp();
22+
expect(phpFactory).toHaveBeenCalledTimes(1);
23+
expect(php).toBeInstanceOf(PHP);
24+
});
25+
26+
it('should return the same PHP instance on subsequent getPrimaryPhp calls', async () => {
27+
const phpFactory = vitest.fn(
28+
async () => new PHP(await loadNodeRuntime(RecommendedPHPVersion))
29+
);
30+
const mgr = new SinglePHPInstanceManager({ phpFactory });
31+
32+
const php1 = await mgr.getPrimaryPhp();
33+
const php2 = await mgr.getPrimaryPhp();
34+
expect(php1).toBe(php2);
35+
expect(phpFactory).toHaveBeenCalledTimes(1);
36+
});
37+
38+
it('should acquire and release the PHP instance', async () => {
39+
const php = new PHP(await loadNodeRuntime(RecommendedPHPVersion));
40+
const mgr = new SinglePHPInstanceManager({ php });
41+
42+
const acquired = await mgr.acquirePHPInstance();
43+
expect(acquired.php).toBe(php);
44+
45+
acquired.reap();
46+
47+
// Should be able to acquire again after reaping
48+
const acquired2 = await mgr.acquirePHPInstance();
49+
expect(acquired2.php).toBe(php);
50+
});
51+
52+
it('should throw an error when trying to acquire twice without reaping', async () => {
53+
const php = new PHP(await loadNodeRuntime(RecommendedPHPVersion));
54+
const mgr = new SinglePHPInstanceManager({ php });
55+
56+
await mgr.acquirePHPInstance();
57+
await expect(() => mgr.acquirePHPInstance()).rejects.toThrowError(
58+
/already acquired/
59+
);
60+
});
61+
62+
it('should throw an error when neither php nor phpFactory is provided', () => {
63+
expect(
64+
() => new SinglePHPInstanceManager({})
65+
).toThrowError(/requires either php or phpFactory/);
66+
});
67+
68+
it('should only call the factory once even with concurrent getPrimaryPhp calls', async () => {
69+
const phpFactory = vitest.fn(
70+
async () => new PHP(await loadNodeRuntime(RecommendedPHPVersion))
71+
);
72+
const mgr = new SinglePHPInstanceManager({ phpFactory });
73+
74+
// Make concurrent calls
75+
const [php1, php2, php3] = await Promise.all([
76+
mgr.getPrimaryPhp(),
77+
mgr.getPrimaryPhp(),
78+
mgr.getPrimaryPhp(),
79+
]);
80+
81+
expect(phpFactory).toHaveBeenCalledTimes(1);
82+
expect(php1).toBe(php2);
83+
expect(php2).toBe(php3);
84+
});
85+
});
86+
87+
describe('PHPProcessManager', () => {
88+
it('should return the primary PHP instance', async () => {
89+
const mgr = new PHPProcessManager({
90+
phpFactory: async () =>
91+
new PHP(await loadNodeRuntime(RecommendedPHPVersion)),
92+
maxPhpInstances: 4,
93+
});
94+
95+
const php = await mgr.getPrimaryPhp();
96+
expect(php).toBeInstanceOf(PHP);
97+
});
98+
99+
it('should spawn new PHP instances', async () => {
100+
const mgr = new PHPProcessManager({
101+
phpFactory: async () =>
102+
new PHP(await loadNodeRuntime(RecommendedPHPVersion)),
103+
maxPhpInstances: 4,
104+
});
105+
106+
const php1 = await mgr.acquirePHPInstance();
107+
expect(php1.php).toBeInstanceOf(PHP);
108+
109+
const php2 = await mgr.acquirePHPInstance();
110+
expect(php1.php).not.toBe(php2.php);
111+
});
112+
113+
it('should not spawn primary PHP until the first acquire call', async () => {
114+
const phpFactory = vitest.fn(
115+
async () => new PHP(await loadNodeRuntime(RecommendedPHPVersion))
116+
);
117+
const mgr = new PHPProcessManager({
118+
phpFactory,
119+
maxPhpInstances: 4,
120+
});
121+
122+
expect(phpFactory).not.toHaveBeenCalled();
123+
await mgr.acquirePHPInstance();
124+
expect(phpFactory).toHaveBeenCalled();
125+
});
126+
127+
it('should refuse to spawn more PHP instances than the maximum (limit=2)', async () => {
128+
const mgr = new PHPProcessManager({
129+
phpFactory: async () =>
130+
new PHP(await loadNodeRuntime(RecommendedPHPVersion)),
131+
maxPhpInstances: 2,
132+
timeout: 100,
133+
});
134+
135+
await mgr.acquirePHPInstance({ considerPrimary: true });
136+
await mgr.acquirePHPInstance({ considerPrimary: true });
137+
await expect(() =>
138+
mgr.acquirePHPInstance({ considerPrimary: true })
139+
).rejects.toThrowError(/Requested more concurrent PHP instances/);
140+
});
141+
142+
it('should refuse to spawn more PHP instances than the maximum (limit=3)', async () => {
143+
const mgr = new PHPProcessManager({
144+
phpFactory: async () =>
145+
new PHP(await loadNodeRuntime(RecommendedPHPVersion)),
146+
maxPhpInstances: 3,
147+
timeout: 100,
148+
});
149+
150+
await mgr.acquirePHPInstance({ considerPrimary: true });
151+
await mgr.acquirePHPInstance({ considerPrimary: true });
152+
await mgr.acquirePHPInstance({ considerPrimary: true });
153+
await expect(() =>
154+
mgr.acquirePHPInstance({ considerPrimary: true })
155+
).rejects.toThrowError(/Requested more concurrent PHP instances/);
156+
});
157+
158+
it('should not start a second PHP instance until the first getInstance() call when the primary instance is busy', async () => {
159+
const phpFactory = vitest.fn(
160+
async () => new PHP(await loadNodeRuntime(RecommendedPHPVersion))
161+
);
162+
const mgr = new PHPProcessManager({
163+
phpFactory,
164+
maxPhpInstances: 5,
165+
});
166+
167+
expect(phpFactory).not.toHaveBeenCalled();
168+
const php1 = await mgr.acquirePHPInstance({ considerPrimary: true });
169+
expect(phpFactory).toHaveBeenCalledTimes(1);
170+
php1.reap();
171+
172+
const php2 = await mgr.acquirePHPInstance({ considerPrimary: true });
173+
expect(phpFactory).toHaveBeenCalledTimes(1);
174+
php2.reap();
175+
176+
await mgr.acquirePHPInstance({ considerPrimary: true });
177+
await mgr.acquirePHPInstance({ considerPrimary: true });
178+
expect(phpFactory).toHaveBeenCalledTimes(3);
179+
});
180+
181+
it('should refuse to spawn two primary PHP instances', async () => {
182+
const mgr = new PHPProcessManager({
183+
phpFactory: async () =>
184+
new PHP(await loadNodeRuntime(RecommendedPHPVersion)),
185+
maxPhpInstances: 5,
186+
});
187+
188+
// A synchronous call. Do not await this promise on purpose.
189+
mgr.getPrimaryPhp();
190+
191+
// No await here, because we want to check if a second,
192+
// synchronous call throws an error if issued before
193+
// the first call completes asynchronously.
194+
try {
195+
mgr.getPrimaryPhp();
196+
} catch (e) {
197+
expect(e).toBeInstanceOf(Error);
198+
expect((e as Error).message).toContain(
199+
'Requested spawning a primary PHP instance'
200+
);
201+
}
202+
});
203+
});

‎packages/php-wasm/node/src/test/php-request-handler.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ describe.each(configsForRequestTests)(
277277
: joinPaths(
278278
new URL(absoluteUrl as string).pathname,
279279
nonExistentFileUri
280-
);
280+
);
281281
expect(response).toEqual({
282282
httpStatusCode: 200,
283283
headers: expect.any(Object),
@@ -989,7 +989,7 @@ describe('PHPRequestHandler – Loopback call', () => {
989989
);
990990
}
991991
const { php, reap } =
992-
await handler.processManager.acquirePHPInstance();
992+
await handler.instanceManager.acquirePHPInstance();
993993
const result = await php.run({
994994
scriptPath: args[1],
995995
env: options.env,

‎packages/php-wasm/node/src/test/php-worker.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ describe('PHP Worker', () => {
3737
* Block the primary PHP instance to ensure run()
3838
* creates a fresh PHP instance.
3939
*/
40-
const { reap } = await handler.processManager.acquirePHPInstance({
40+
const { reap } = await handler.instanceManager.acquirePHPInstance({
4141
considerPrimary: instanceType === 'primary',
4242
});
4343
try {

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,22 @@ 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 type {
28+
PHPInstanceManager,
29+
AcquiredPHP,
30+
/**
31+
* Backwards compatibility alias.
32+
*/
33+
AcquiredPHP as SpawnedPHP,
34+
} from './php-instance-manager';
35+
export { SinglePHPInstanceManager } from './single-php-instance-manager';
36+
export type { SinglePHPInstanceManagerOptions } from './single-php-instance-manager';
2737
export { PHPProcessManager } from './php-process-manager';
2838
export type {
2939
MaxPhpInstancesError,
3040
PHPFactory,
3141
PHPFactoryOptions,
3242
ProcessManagerOptions,
33-
SpawnedPHP,
3443
} from './php-process-manager';
3544

3645
export { PHPResponse, StreamedPHPResponse } from './php-response';

‎packages/php-wasm/universal/src/lib/php-instance-manager.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import type { PHP } from './php';
2+
3+
/**
4+
* Result of acquiring a PHP instance.
5+
* The `reap` function should be called when done with the instance.
6+
*/
7+
export interface AcquiredPHP {
8+
php: PHP;
9+
/**
10+
* Release the PHP instance back to the pool (for multi-instance managers)
11+
* or mark it as idle (for single-instance managers).
12+
*/
13+
reap: () => void;
14+
}
15+
16+
/**
17+
* Minimal interface for managing PHP instances.
18+
*
19+
* This interface allows PHPRequestHandler to work with different
20+
* instance management strategies:
21+
* - PHPProcessManager: Multiple PHP instances with concurrency control
22+
* - SinglePHPInstanceManager: Single PHP instance for CLI contexts
23+
*/
24+
export interface PHPInstanceManager extends AsyncDisposable {
25+
/**
26+
* Get the primary PHP instance.
27+
* This instance is persistent and never killed.
28+
*/
29+
getPrimaryPhp(): Promise<PHP>;
30+
31+
/**
32+
* Acquire a PHP instance for processing a request.
33+
*
34+
* @param options.considerPrimary - Whether the primary instance can be used.
35+
* Set to false for operations that would corrupt the primary (e.g., CLI commands).
36+
* @returns An acquired PHP instance with a reap function.
37+
*/
38+
acquirePHPInstance(options?: {
39+
considerPrimary?: boolean;
40+
}): Promise<AcquiredPHP>;
41+
}

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

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { AcquireTimeoutError, Semaphore } from '@php-wasm/util';
22
import type { PHP } from './php';
3+
import type { PHPInstanceManager, AcquiredPHP } from './php-instance-manager';
34

45
export type PHPFactoryOptions = {
56
isPrimary: boolean;
@@ -33,11 +34,6 @@ export interface ProcessManagerOptions {
3334
phpFactory?: PHPFactory;
3435
}
3536

36-
export interface SpawnedPHP {
37-
php: PHP;
38-
reap: () => void;
39-
}
40-
4137
export class MaxPhpInstancesError extends Error {
4238
constructor(limit: number) {
4339
super(
@@ -71,16 +67,16 @@ export class MaxPhpInstancesError extends Error {
7167
* requests requires extra time to spin up a few PHP instances. This is a more
7268
* resource-friendly tradeoff than keeping 5 idle instances at all times.
7369
*/
74-
export class PHPProcessManager implements AsyncDisposable {
70+
export class PHPProcessManager implements PHPInstanceManager {
7571
private primaryPhp?: PHP;
76-
private primaryPhpPromise?: Promise<SpawnedPHP>;
72+
private primaryPhpPromise?: Promise<AcquiredPHP>;
7773
private primaryIdle = true;
78-
private nextInstance: Promise<SpawnedPHP> | null = null;
74+
private nextInstance: Promise<AcquiredPHP> | null = null;
7975
/**
8076
* All spawned PHP instances, including the primary PHP instance.
8177
* Used for bookkeeping and reaping all instances on dispose.
8278
*/
83-
private allInstances: Promise<SpawnedPHP>[] = [];
79+
private allInstances: Promise<AcquiredPHP>[] = [];
8480
private phpFactory?: PHPFactory;
8581
private maxPhpInstances: number;
8682
private semaphore: Semaphore;
@@ -147,7 +143,7 @@ export class PHPProcessManager implements AsyncDisposable {
147143
considerPrimary = false,
148144
}: {
149145
considerPrimary?: boolean;
150-
} = {}): Promise<SpawnedPHP> {
146+
} = {}): Promise<AcquiredPHP> {
151147
/**
152148
* First and foremost, make sure we have the primary PHP instance in place.
153149
* We may not actually acquire it. We just need it to exist.
@@ -178,7 +174,7 @@ export class PHPProcessManager implements AsyncDisposable {
178174
* budget left to optimistically start spawning the next
179175
* instance.
180176
*/
181-
const spawnedPhp =
177+
const acquiredPHP =
182178
this.nextInstance || this.spawn({ isPrimary: false });
183179

184180
/**
@@ -191,7 +187,7 @@ export class PHPProcessManager implements AsyncDisposable {
191187
} else {
192188
this.nextInstance = null;
193189
}
194-
return await spawnedPhp;
190+
return await acquiredPHP;
195191
}
196192

197193
/**
@@ -200,7 +196,7 @@ export class PHPProcessManager implements AsyncDisposable {
200196
* add the spawn promise to the allInstances array without waiting
201197
* for PHP to spawn.
202198
*/
203-
private spawn(factoryArgs: PHPFactoryOptions): Promise<SpawnedPHP> {
199+
private spawn(factoryArgs: PHPFactoryOptions): Promise<AcquiredPHP> {
204200
if (factoryArgs.isPrimary && this.allInstances.length > 0) {
205201
throw new Error(
206202
'Requested spawning a primary PHP instance when another primary instance already started spawning.'
@@ -230,7 +226,9 @@ export class PHPProcessManager implements AsyncDisposable {
230226
/**
231227
* Actually acquires the lock and spawns a new PHP instance.
232228
*/
233-
private async doSpawn(factoryArgs: PHPFactoryOptions): Promise<SpawnedPHP> {
229+
private async doSpawn(
230+
factoryArgs: PHPFactoryOptions
231+
): Promise<AcquiredPHP> {
234232
let release: () => void;
235233
try {
236234
release = await this.semaphore.acquire();

0 commit comments

Comments
 (0)