Skip to content

Commit d160270

Browse files
Fix unmount of nested mounts in PHP runtime hotswap (#2859)
## Motivation for the change, related issues Today, if we nest a NODEFS mount inside another NODEFS mount, hot-swapping the PHP runtime fails when we attempt to unmount a nested mount from a parent that has already been unmounted. This PR fixes that issue. cc @adamziel who [mentioned an issue](#2836 (comment)) that looked like this one. ## Implementation details This PR changes PHP runtime hot-swapping to unmount mounts in reverse order, and the attempted unmount no longer fails. ## Testing Instructions (or ideally a Blueprint) - CI
1 parent e441e31 commit d160270

File tree

2 files changed

+69
-7
lines changed

2 files changed

+69
-7
lines changed

packages/php-wasm/node/src/test/rotate-php-runtime.spec.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,59 @@ describe.each([true, false])(
213213
).toBe('plugin-3');
214214
});
215215

216+
it('Preserves a nested NODEFS mount through PHP runtime recreation', async () => {
217+
// Rotate the PHP runtime
218+
const recreateRuntimeSpy = vitest.fn(recreateRuntime);
219+
220+
const php = new PHP(await recreateRuntime());
221+
php.enableRuntimeRotation({
222+
recreateRuntime: recreateRuntimeSpy,
223+
maxRequests: 1,
224+
});
225+
226+
// Create a temporary directory and a file in it
227+
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'temp-'));
228+
const dirToMountAsParent = path.join(tempDir, 'parent');
229+
fs.mkdirSync(dirToMountAsParent);
230+
const dirToMountAsNested = path.join(tempDir, 'nested');
231+
fs.mkdirSync(dirToMountAsNested);
232+
233+
// Mount the parent and nested child directories
234+
await php.mount(
235+
'/parent',
236+
createNodeFsMountHandler(dirToMountAsParent)
237+
);
238+
await php.mount(
239+
'/parent/nested',
240+
createNodeFsMountHandler(dirToMountAsNested)
241+
);
242+
243+
const childFile = path.join(dirToMountAsNested, 'nested.php');
244+
const nestedPhpCode = `<?php echo "nested file";`;
245+
fs.writeFileSync(childFile, nestedPhpCode);
246+
const parentFileThatReferencesChildFile = path.join(
247+
dirToMountAsParent,
248+
'test.php'
249+
);
250+
const parentPhpCode = `<?php require_once __DIR__ . '/nested/nested.php';`;
251+
fs.writeFileSync(parentFileThatReferencesChildFile, parentPhpCode);
252+
253+
// Confirm output based on nested NODEFS mount is the same before and after rotation.
254+
const scriptPath = '/parent/test.php';
255+
const firstResult = await php.run({ scriptPath });
256+
expect(firstResult.text).toBe('nested file');
257+
const secondResult = await php.run({ scriptPath });
258+
expect(secondResult.text).toBe('nested file');
259+
260+
expect(recreateRuntimeSpy).toHaveBeenCalledTimes(1);
261+
262+
// Infer that the nested NODEFS mounts are not lost
263+
expect(php.readFileAsText('/parent/nested/nested.php')).toBe(
264+
nestedPhpCode
265+
);
266+
expect(php.readFileAsText(scriptPath)).toBe(parentPhpCode);
267+
});
268+
216269
it('Free up the available PHP memory', async () => {
217270
const freeMemory = (php: PHP) =>
218271
php[__private__dont__use].HEAPU32.reduce(

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,11 +1398,20 @@ export class PHP implements Disposable {
13981398
const oldCWD = oldFS.cwd();
13991399
oldFS.chdir('/');
14001400

1401-
// Unmount all the mount handlers
1402-
const mountHandlers: { mountHandler: MountHandler; vfsPath: string }[] =
1403-
[];
1404-
for (const [vfsPath, mount] of Object.entries(this.#mounts)) {
1405-
mountHandlers.push({ mountHandler: mount.mountHandler, vfsPath });
1401+
// Remember mounts to apply to new runtime
1402+
const mountHandlersToReapplyInOrder = Object.entries(this.#mounts).map(
1403+
([vfsPath, mount]) => ({
1404+
mountHandler: mount.mountHandler,
1405+
vfsPath,
1406+
})
1407+
);
1408+
1409+
// Unmount all the mount handlers in reverse order because each nested
1410+
// mount depends upon the parent mount which preceded it.
1411+
const mountsToUnmountInReverseOrder = Object.values(
1412+
this.#mounts
1413+
).reverse();
1414+
for (const mount of mountsToUnmountInReverseOrder) {
14061415
await mount.unmount();
14071416
}
14081417

@@ -1441,8 +1450,8 @@ export class PHP implements Disposable {
14411450
}
14421451
}
14431452

1444-
// Re-mount all the mount handlers
1445-
for (const { mountHandler, vfsPath } of mountHandlers) {
1453+
// Re-mount all the mount handlers in order
1454+
for (const { mountHandler, vfsPath } of mountHandlersToReapplyInOrder) {
14461455
this.mkdir(vfsPath);
14471456
await this.mount(vfsPath, mountHandler);
14481457
}

0 commit comments

Comments
 (0)