Run more Playground CLI tests on Windows and macOS

.github/workflows/ci.yml

@@ -135,7 +135,7 @@ jobs:
                 # It would be great to fix the issue and add macOS testing here.
                 # In the meantime, many of the current developers test locally on macOS.
                 # @TODO: Fix issues and re-enable macOS testing.
-                os: [ubuntu-latest, windows-latest]
+                os: [ubuntu-latest, windows-latest, macos-latest]
         continue-on-error: true
         runs-on: ${{ matrix.os }}
         name: 'test-playground-cli (${{ matrix.os }})'

packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts

@@ -1,10 +1,24 @@
 import { logger } from '@php-wasm/logger';
 import { openSync, closeSync } from 'fs';
-
-type NativeFlockSync = (
-	fd: number,
-	flags: 'sh' | 'ex' | 'shnb' | 'exnb' | 'un'
-) => void;
+import { platform } from 'os';
+
+export type NativeLockingAPI = {
+	flockSync: (
+		fd: number,
+		flags: 'sh' | 'ex' | 'shnb' | 'exnb' | 'un',
+		arg?: number | undefined
+	) => void;
+	fcntlSync: (
+		fd: number,
+		command: 'getfd' | 'setfd' | 'setlk' | 'getlk' | 'setlkw',
+		arg: number | undefined
+	) => number;
+	constants: {
+		F_WRLCK: number;
+		F_RDLCK: number;
+		F_UNLCK: number;
+	};
+};
 
 import type {
 	FileLockManager,
@@ -20,7 +34,7 @@ type LockMode = 'exclusive' | 'shared' | 'unlock';
 type NativeLock = {
 	fd: number;
 	mode: LockMode;
-	nativeFlockSync: NativeFlockSync;
+	nativeLockingAPI: NativeLockingAPI;
 };
 
 type LockedRange = RequestedRangeLock & {
@@ -36,7 +50,7 @@ const MAX_64BIT_OFFSET = BigInt(2n ** 64n - 1n);
  * It provides methods for locking and unlocking files, as well as finding conflicting locks.
  */
 export class FileLockManagerForNode implements FileLockManager {
-	nativeFlockSync: NativeFlockSync;
+	nativeLockingAPI: NativeLockingAPI;
 	locks: Map<string, FileLock>;
 
 	/**
@@ -45,11 +59,22 @@ export class FileLockManagerForNode implements FileLockManager {
 	 * @param nativeFlockSync A synchronous flock() function to lock files via the host OS.
 	 */
 	constructor(
-		nativeFlockSync: NativeFlockSync = function flockSyncNoOp() {
-			/* do nothing */
+		nativeLockingAPI: NativeLockingAPI = {
+			flockSync: function flockSyncNoOp() {
+				/* do nothing */
+			},
+			fcntlSync: function fcntlSyncNoOp() {
+				/* do nothing */
+				return 0;
+			},
+			constants: {
+				F_WRLCK: 1,
+				F_RDLCK: 2,
+				F_UNLCK: 3,
+			},
 		}
 	) {
-		this.nativeFlockSync = nativeFlockSync;
+		this.nativeLockingAPI = nativeLockingAPI;
 		this.locks = new Map();
 	}
 
@@ -70,7 +95,7 @@ export class FileLockManagerForNode implements FileLockManager {
 			const maybeLock = FileLock.maybeCreate(
 				path,
 				op.type,
-				this.nativeFlockSync
+				this.nativeLockingAPI
 			);
 			if (maybeLock === undefined) {
 				return false;
@@ -105,7 +130,7 @@ export class FileLockManagerForNode implements FileLockManager {
 			const maybeLock = FileLock.maybeCreate(
 				path,
 				requestedLock.type,
-				this.nativeFlockSync
+				this.nativeLockingAPI
 			);
 			if (maybeLock === undefined) {
 				return false;
@@ -202,16 +227,30 @@ export class FileLock {
 	static maybeCreate(
 		path: string,
 		mode: Exclude<WholeFileLock['type'], 'unlocked'>,
-		nativeFlockSync: NativeFlockSync
+		nativeLockingAPI: NativeLockingAPI
 	): FileLock | undefined {
 		let fd;
 		try {
 			fd = openSync(path, 'a+');
 
-			const flockFlags = mode === 'exclusive' ? 'exnb' : 'shnb';
-			nativeFlockSync(fd, flockFlags);
+			if (platform() === 'win32') {
+				// Windows does not support downgrading or upgrading
+				// an existing lock in-place. Instead, the existing lock must be
+				// released before attempting to acquire a new lock at the desired level.
+				// Since SQLite expects POSIX-like behavior with fcntl() which allows
+				// upgrading and downgrading an existing lock,
+				// we cannot afford the possibility of losing the current lock in Windows.
+				// Therefore, we always request an exclusive lock on Windows.
+				nativeLockingAPI.flockSync(fd, 'exnb');
+			} else {
+				// TODO: Update locking to obtain native locks for both fcntl() and flock()
+				nativeLockingAPI.flockSync(
+					fd,
+					mode === 'exclusive' ? 'exnb' : 'shnb'
+				);
+			}
 
-			const nativeLock: NativeLock = { fd, mode, nativeFlockSync };
+			const nativeLock: NativeLock = { fd, mode, nativeLockingAPI };
 			return new FileLock(nativeLock);
 		} catch {
 			if (fd !== undefined) {
@@ -598,13 +637,33 @@ export class FileLock {
 			return true;
 		}
 
-		const flockFlags =
-			(requiredNativeLockType === 'exclusive' && 'exnb') ||
-			(requiredNativeLockType === 'shared' && 'shnb') ||
-			'un';
-
 		try {
-			this.nativeLock.nativeFlockSync(this.nativeLock.fd, flockFlags);
+			if (platform() === 'win32') {
+				// Windows does not support downgrading or upgrading
+				// an existing lock in-place. Instead, the existing lock must be
+				// released before attempting to acquire a new lock at the desired level.
+				// Since SQLite expects POSIX-like behavior with fcntl() which allows
+				// upgrading and downgrading an existing lock,
+				// we cannot afford the possibility of losing the current lock in Windows.
+				// Therefore, we always request an exclusive lock on Windows
+				// and hold that lock as-is until the lock needs to be released entirely.
+				if (requiredNativeLockType === 'unlock') {
+					this.nativeLock.nativeLockingAPI.flockSync(
+						this.nativeLock.fd,
+						'un'
+					);
+				}
+			} else {
+				const flags =
+					(requiredNativeLockType === 'exclusive' && 'exnb') ||
+					(requiredNativeLockType === 'shared' && 'shnb') ||
+					'un';
+				this.nativeLock.nativeLockingAPI.flockSync(
+					this.nativeLock.fd,
+					flags
+				);
+			}
+
 			this.nativeLock.mode = requiredNativeLockType;
 			return true;
 		} catch {

packages/php-wasm/node/src/test/file-lock-manager-for-node.spec.ts

@@ -4,7 +4,7 @@ import { fork } from 'child_process';
 import type { ChildProcess } from 'child_process';
 import { join } from 'path';
 import type { WholeFileLockOp } from '../lib/file-lock-manager';
-import { flockSync as nativeFlockSync } from 'fs-ext';
+import fsExt from 'fs-ext';
 
 const TEST_FILE1 = new URL('test1.txt', import.meta.url).pathname;
 const TEST_FILE2 = new URL('test2.txt', import.meta.url).pathname;
@@ -13,7 +13,7 @@ describe('FileLockManagerForNode', () => {
 	let lockManager: FileLockManagerForNode;
 
 	beforeEach(() => {
-		lockManager = new FileLockManagerForNode(nativeFlockSync);
+		lockManager = new FileLockManagerForNode(fsExt);
 		writeFileSync(TEST_FILE1, `test file 1 for ${import.meta.url}`);
 		writeFileSync(TEST_FILE2, `test file 2 for ${import.meta.url}`);
 	});

packages/playground/cli/src/run-cli.ts

@@ -47,7 +47,6 @@ import { BlueprintsV2Handler } from './blueprints-v2/blueprints-v2-handler';
 import { BlueprintsV1Handler } from './blueprints-v1/blueprints-v1-handler';
 import { startBridge } from '@php-wasm/xdebug-bridge';
 import path from 'path';
-import os from 'os';
 import {
 	cleanupStalePlaygroundTempDirs,
 	createPlaygroundCliTempDir,
@@ -626,22 +625,15 @@ export async function runCLI(args: RunCLIArgs): Promise<RunCLIServer | void> {
 
 	// Declare file lock manager outside scope of startServer
 	// so we can look at it when debugging request handling.
-	const nativeFlockSync =
-		os.platform() === 'win32'
-			? // @TODO: Enable fs-ext here when it works with Windows.
-				undefined
-			: await import('fs-ext')
-					.then((m) => m.flockSync)
-					.catch(() => {
-						logger.warn(
-							'The fs-ext package is not installed. ' +
-								'Internal file locking will not be integrated with ' +
-								'host OS file locking.'
-						);
-						return undefined;
-					});
-	const fileLockManager = new FileLockManagerForNode(nativeFlockSync);
-
+	const nativeLockingAPI = await import('fs-ext').catch(() => {
+		logger.warn(
+			'The fs-ext package is not installed. ' +
+				'Internal file locking will not be integrated with ' +
+				'host OS file locking.'
+		);
+		return undefined;
+	});
+	const fileLockManager = new FileLockManagerForNode(nativeLockingAPI);
 	let wordPressReady = false;
 	let isFirstRequest = true;