[PHP] Keep OPFS mounts attached when final journal flushes fail

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

@@ -66,7 +66,12 @@ export {
 	LEGACY_PHP_INI_CONTENT,
 	LEGACY_PHP_INI_PATH,
 } from './legacy-php-ini';
-export { PHP, __private__dont__use, PHPExecutionFailureError } from './php';
+export {
+	MountStillActiveError,
+	PHP,
+	__private__dont__use,
+	PHPExecutionFailureError,
+} from './php';
 export type { MountHandler, UnmountFunction } from './php';
 export { loadPHPRuntime, popLoadedRuntime } from './load-php-runtime';
 export type { Emscripten } from './emscripten-types';

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

@@ -1,5 +1,5 @@
 import { describe, expect, it, vi } from 'vitest';
-import { __private__dont__use, PHP } from './php';
+import { __private__dont__use, MountStillActiveError, PHP } from './php';
 
 describe('PHP mounts', () => {
 	it('forgets mount tracking even when the unmount callback fails', async () => {
@@ -8,9 +8,9 @@ describe('PHP mounts', () => {
 		//
 		// 1. Invokes the underlying unmount callback returned by the
 		//    mount handler.
-		// 2. Deletes the entry from `#mounts` in a `finally` block so the
-		//    JS-side bookkeeping stays authoritative even if the
-		//    underlying filesystem unmount throws.
+		// 2. Deletes the entry from `#mounts` after ordinary failures so the
+		//    JS-side bookkeeping stays authoritative when the underlying
+		//    filesystem did not explicitly report that it remains mounted.
 		//
 		// `#mounts` is not observable from outside the class, so we
 		// verify the cleanup transitively through `hotSwapPHPRuntime`,
@@ -27,7 +27,7 @@ describe('PHP mounts', () => {
 		//
 		// - `unmountCallback` must have been called exactly once across
 		//   both the explicit `unmount()` call and the subsequent
-		//   `hotSwapPHPRuntime` attempt. Any stale `#mounts` entry would
+		//   `hotSwapPHPRuntime` attempt. Any stale ordinary-failure entry would
 		//   bump the count to 2.
 		const php = new PHP();
 		(php as any)[__private__dont__use] = {
@@ -55,4 +55,33 @@ describe('PHP mounts', () => {
 		);
 		expect(unmountCallback).toHaveBeenCalledTimes(1);
 	});
+
+	it('retains mount tracking when the unmount reports it is still active', async () => {
+		const php = new PHP();
+		(php as any)[__private__dont__use] = {
+			FS: {
+				chdir: vi.fn(),
+				cwd: vi.fn(() => '/'),
+				lookupPath: vi.fn(() => ({})),
+				readdir: vi.fn(() => ['.', '..']),
+			},
+			spawnProcess: undefined,
+		};
+		const flushError = new Error('flush failed');
+		const activeError = new MountStillActiveError(flushError);
+		const unmountCallback = vi
+			.fn()
+			.mockRejectedValueOnce(activeError)
+			.mockResolvedValueOnce(undefined);
+		const unmount = await php.mount('/mounted', async () => {
+			return unmountCallback;
+		});
+
+		await expect(unmount()).rejects.toBe(activeError);
+		await expect(php.hotSwapPHPRuntime(0 as any)).rejects.toThrow(
+			'Runtime with id 0 not found'
+		);
+
+		expect(unmountCallback).toHaveBeenCalledTimes(2);
+	});
 });

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

@@ -48,6 +48,23 @@ export class PHPExecutionFailureError extends Error {
 }
 
 export type UnmountFunction = (() => Promise<any>) | (() => any);
+
+/**
+ * Signals that an unmount failed before the mount was detached.
+ *
+ * Only this error guarantees that the mount remains active and its teardown can
+ * be retried, so registries may retain it. `cause` is the underlying failure that
+ * prevented the unmount. Ordinary unmount errors make no such guarantee.
+ */
+export class MountStillActiveError extends Error {
+	constructor(cause: unknown) {
+		super('The filesystem could not be flushed and remains mounted.', {
+			cause,
+		});
+		this.name = 'MountStillActiveError';
+	}
+}
+
 export type MountHandler = (
 	php: PHP,
 	FS: Emscripten.RootFS,
@@ -1533,6 +1550,10 @@ export class PHP implements Disposable {
 	/**
 	 * Mounts a filesystem to a given path in the PHP filesystem.
 	 *
+	 * The returned unmount function removes the mount from runtime-rotation
+	 * tracking on success or ordinary failure. `MountStillActiveError` leaves it
+	 * tracked because the handler guarantees the mount remains live and retryable.
+	 *
 	 * @param  virtualFSPath - Where to mount it in the PHP virtual filesystem.
 	 * @param  mountHandler - The mount handler to use.
 	 * @return Unmount function to unmount the filesystem.
@@ -1551,13 +1572,17 @@ export class PHP implements Disposable {
 			unmount: async () => {
 				try {
 					await unmountCallback();
-				} finally {
-					// JS mount tracking is authoritative. Even if the
-					// underlying filesystem unmount fails, forget this entry
-					// so later runtime swaps and remounts cannot reuse a stale
-					// mount handler.
+				} catch (error) {
+					if (error instanceof MountStillActiveError) {
+						throw error;
+					}
+					// Unless the callback guarantees that the mount remains live,
+					// retaining it could retry stale teardown state during a later
+					// runtime swap.
 					delete this.#mounts[virtualFSPath];
+					throw error;
 				}
+				delete this.#mounts[virtualFSPath];
 			},
 		};
 		this.#mounts[virtualFSPath] = mountObject;

packages/php-wasm/web/src/lib/directory-handle-mount.spec.ts

@@ -152,6 +152,38 @@ describe('journalFSEventsToOpfs', () => {
 		await firstFlush;
 	});
 
+	it('waits for an in-flight flush when discarding an incomplete mount', async () => {
+		const writeStarted = deferred<void>();
+		const releaseWrite = deferred<void>();
+		const { FS, files, php, removeEventListener } = createFakePhp();
+		const opfsRoot = new MemoryDirectoryHandle('root', async () => {
+			writeStarted.resolve();
+			await releaseWrite.promise;
+		});
+		const mount = journalFSEventsToOpfs(
+			php,
+			opfsRoot as unknown as FileSystemDirectoryHandle,
+			'/wordpress'
+		);
+		files.set('/wordpress/file.txt', encode('saved'));
+		FS.write({ path: '/wordpress/file.txt' });
+		void mount.flush();
+		await writeStarted.promise;
+
+		let discardSettled = false;
+		const discard = mount.discard().then(() => {
+			discardSettled = true;
+		});
+		await Promise.resolve();
+
+		expect(removeEventListener).toHaveBeenCalled();
+		expect(discardSettled).toBe(false);
+
+		releaseWrite.resolve();
+		await discard;
+		expect(discardSettled).toBe(true);
+	});
+
 	it('processes new writes on a subsequent flush after the previous flush succeeded', async () => {
 		// Regression guard for the `flushPromise` single-flight reset on the
 		// success path. If `flushPromise` were not cleared in the `.finally()`
@@ -248,13 +280,52 @@ describe('journalFSEventsToOpfs', () => {
 		expect(FS.write).toBe(originalWrite);
 	});
 
-	it('removes listeners even when unmount flush fails', async () => {
+	it('flushes writes queued as the in-flight flush settles before unmounting', async () => {
+		const writeStarted = deferred<void>();
+		const releaseWrite = deferred<void>();
+		let writeCount = 0;
+		const { FS, files, php } = createFakePhp();
+		const opfsRoot = new MemoryDirectoryHandle('root', async () => {
+			writeCount++;
+			if (writeCount === 1) {
+				writeStarted.resolve();
+				await releaseWrite.promise;
+			}
+		});
+		const mount = journalFSEventsToOpfs(
+			php,
+			opfsRoot as unknown as FileSystemDirectoryHandle,
+			'/wordpress'
+		);
+
+		files.set('/wordpress/first.txt', encode('first'));
+		FS.write({ path: '/wordpress/first.txt' });
+		const inFlightFlush = mount.flush();
+		await writeStarted.promise;
+		const enqueueAtFlushBoundary = inFlightFlush.then(() => {
+			files.set('/wordpress/second.txt', encode('second'));
+			FS.write({ path: '/wordpress/second.txt' });
+		});
+		const unmount = mount.unmount();
+
+		releaseWrite.resolve();
+		await Promise.all([enqueueAtFlushBoundary, unmount]);
+
+		expect(decode(opfsRoot.files.get('first.txt')!.bytes)).toBe('first');
+		expect(decode(opfsRoot.files.get('second.txt')!.bytes)).toBe('second');
+	});
+
+	it('keeps listeners until a failed unmount flush can be retried', async () => {
 		const flushError = new Error('flush failed');
+		let shouldFail = true;
 		const { FS, addEventListener, files, php, removeEventListener } =
 			createFakePhp();
 		const originalWrite = FS.write;
 		const opfsRoot = new MemoryDirectoryHandle('root', () => {
-			throw flushError;
+			if (shouldFail) {
+				shouldFail = false;
+				throw flushError;
+			}
 		});
 		const mount = journalFSEventsToOpfs(
 			php,
@@ -271,7 +342,14 @@ describe('journalFSEventsToOpfs', () => {
 		files.set('/wordpress/file.txt', encode('saved'));
 		FS.write({ path: '/wordpress/file.txt' });
 
-		await expect(mount.unmount()).rejects.toBe(flushError);
+		await expect(mount.unmount()).rejects.toMatchObject({
+			name: 'MountStillActiveError',
+			cause: flushError,
+		});
+		expect(removeEventListener).not.toHaveBeenCalled();
+		expect(FS.write).not.toBe(originalWrite);
+
+		await mount.unmount();
 		expect(removeEventListener).toHaveBeenCalledWith(
 			'filesystem.write',
 			filesystemWriteListener
@@ -327,11 +405,37 @@ describe('journalFSEventsToOpfs', () => {
 		FS.write({ path: '/wordpress/file.txt' });
 		await expect(mount.flush()).rejects.toThrow('temporary flush failure');
 
-		files.set('/wordpress/file.txt', encode('second'));
-		FS.write({ path: '/wordpress/file.txt' });
 		await mount.flush();
 
-		expect(decode(opfsRoot.files.get('file.txt')!.bytes)).toBe('second');
+		expect(decode(opfsRoot.files.get('file.txt')!.bytes)).toBe('first');
+	});
+
+	it('retries only the failed suffix of a partially completed batch', async () => {
+		let writeCount = 0;
+		const { FS, files, php } = createFakePhp();
+		const opfsRoot = new MemoryDirectoryHandle('root', () => {
+			writeCount++;
+			if (writeCount === 2) {
+				throw new Error('second write failed');
+			}
+		});
+		const mount = journalFSEventsToOpfs(
+			php,
+			opfsRoot as unknown as FileSystemDirectoryHandle,
+			'/wordpress'
+		);
+
+		files.set('/wordpress/first.txt', encode('first'));
+		files.set('/wordpress/second.txt', encode('second'));
+		FS.write({ path: '/wordpress/first.txt' });
+		FS.write({ path: '/wordpress/second.txt' });
+
+		await expect(mount.flush()).rejects.toThrow('second write failed');
+		await mount.flush();
+
+		expect(writeCount).toBe(3);
+		expect(decode(opfsRoot.files.get('first.txt')!.bytes)).toBe('first');
+		expect(decode(opfsRoot.files.get('second.txt')!.bytes)).toBe('second');
 	});
 
 	it('fails explicit flushes that never settle instead of hanging', async () => {
@@ -420,6 +524,57 @@ describe('createDirectoryHandleMountHandler', () => {
 		expect(opfsRoot.files.has('autoload.php')).toBe(false);
 	});
 
+	it('discards the journal and preserves the copy error when initial sync fails', async () => {
+		const copyError = new Error('initial copy failed');
+		const flushError = new Error('rollback flush failed');
+		const { FS, dispatchEvent, files, php, removeEventListener } =
+			createFakePhp();
+		const originalWrite = FS.write;
+		FS.readdir.mockReturnValue(['.', '..', 'bad.txt']);
+		files.set('/wordpress/bad.txt', encode('bad'));
+		let writeCount = 0;
+		const opfsRoot = new MemoryDirectoryHandle('root', () => {
+			writeCount++;
+			if (writeCount === 1) {
+				files.set('/wordpress/live.txt', encode('live'));
+				FS.write({ path: '/wordpress/live.txt' });
+				dispatchEvent('filesystem.write');
+				throw copyError;
+			}
+			throw flushError;
+		});
+		const loggerError = vi
+			.spyOn(logger, 'error')
+			.mockImplementation(() => {});
+		const mountHandler = createDirectoryHandleMountHandler(
+			opfsRoot as unknown as FileSystemDirectoryHandle,
+			{
+				initialSync: { direction: 'memfs-to-opfs' },
+			}
+		);
+
+		try {
+			await expect(
+				mountHandler(php, FS as any, '/wordpress')
+			).rejects.toMatchObject({ cause: copyError });
+			expect(loggerError).toHaveBeenCalledWith(
+				'OPFS flush failed while discarding a mount',
+				flushError
+			);
+			expect(removeEventListener).toHaveBeenCalledWith(
+				'request.end',
+				expect.any(Function)
+			);
+			expect(removeEventListener).toHaveBeenCalledWith(
+				'filesystem.write',
+				expect.any(Function)
+			);
+			expect(FS.write).toBe(originalWrite);
+		} finally {
+			loggerError.mockRestore();
+		}
+	});
+
 	it('flushes changes made while the initial MEMFS to OPFS sync is still running', async () => {
 		let changedDuringInitialSync = false;
 		let mount: { flush(): Promise<void> } | undefined;