From fb678b2955b95ccffbf24c744bb4ce2b5fee54c8 Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Sun, 23 Nov 2025 18:07:22 -0500 Subject: [PATCH 1/5] Try re-enabling Playground CLI tests for macOS again --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ab62fbb26..f5a9d7e66d 100644 --- a/.github/workflows/ci.yml +++ b/.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 }})' From c705c329f6ad2bdcc07c9563921f5d29ee4683d1 Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Sun, 23 Nov 2025 19:10:31 -0500 Subject: [PATCH 2/5] Fix native file locking in Windows --- .../src/lib/file-lock-manager-for-node.ts | 113 ++++++++++++++---- packages/playground/cli/src/run-cli.ts | 47 +++----- 2 files changed, 109 insertions(+), 51 deletions(-) diff --git a/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts b/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts index 0a740dba59..5a28a5cfa0 100644 --- a/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts +++ b/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; /** @@ -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,32 @@ export class FileLock { static maybeCreate( path: string, mode: Exclude, - 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, 'ex'); + } else { + nativeLockingAPI.fcntlSync( + fd, + 'setlk', + mode === 'exclusive' + ? nativeLockingAPI.constants.F_WRLCK + : nativeLockingAPI.constants.F_RDLCK + ); + } - const nativeLock: NativeLock = { fd, mode, nativeFlockSync }; + const nativeLock: NativeLock = { fd, mode, nativeLockingAPI }; return new FileLock(nativeLock); } catch { if (fd !== undefined) { @@ -598,13 +639,41 @@ 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 { constants } = this.nativeLock.nativeLockingAPI; + const flags = + (requiredNativeLockType === 'exclusive' && + constants.F_WRLCK) || + (requiredNativeLockType === 'shared' && + constants.F_RDLCK) || + constants.F_UNLCK; + + // TODO: Update locking to obtain native locks for both fcntl() and flock() + // operations. On Linux, this is important because fcntl() locks do not conflict + // with flock() locks and vice versa. On macOS, fcntl() and flock() locks can conflict. + this.nativeLock.nativeLockingAPI.fcntlSync( + this.nativeLock.fd, + 'setlk', + flags + ); + } + this.nativeLock.mode = requiredNativeLockType; return true; } catch { diff --git a/packages/playground/cli/src/run-cli.ts b/packages/playground/cli/src/run-cli.ts index 92a5b67529..2835c4dfe5 100644 --- a/packages/playground/cli/src/run-cli.ts +++ b/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, @@ -621,22 +620,15 @@ export async function runCLI(args: RunCLIArgs): Promise { // 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; @@ -651,13 +643,13 @@ export async function runCLI(args: RunCLIArgs): Promise { const targetWorkerCount = args.command === 'server' - ? args.experimentalMultiWorker ?? 1 + ? (args.experimentalMultiWorker ?? 1) : 1; const totalWorkersToSpawn = args.command === 'server' ? // Account for the initial worker - // which is discarded by the server after setup. - targetWorkerCount + 1 + // which is discarded by the server after setup. + targetWorkerCount + 1 : targetWorkerCount; const processIdSpaceLength = Math.floor( @@ -675,9 +667,8 @@ export async function runCLI(args: RunCLIArgs): Promise { * because we don't have to create or maintain multiple copies of the same files. */ const tempDirNameDelimiter = '-playground-cli-site-'; - const nativeDir = await createPlaygroundCliTempDir( - tempDirNameDelimiter - ); + const nativeDir = + await createPlaygroundCliTempDir(tempDirNameDelimiter); logger.debug(`Native temp dir for VFS root: ${nativeDir.path}`); const IDEConfigName = 'WP Playground CLI - Listen for Xdebug'; @@ -948,9 +939,8 @@ export async function runCLI(args: RunCLIArgs): Promise { try { const workers = await promisedWorkers; - const fileLockManagerPort = await exposeFileLockManager( - fileLockManager - ); + const fileLockManagerPort = + await exposeFileLockManager(fileLockManager); // NOTE: Using a free-standing block to isolate initial boot vars // while keeping the logic inline. @@ -1022,9 +1012,8 @@ export async function runCLI(args: RunCLIArgs): Promise { initialWorkerProcessIdSpace + index * processIdSpaceLength; - const fileLockManagerPort = await exposeFileLockManager( - fileLockManager - ); + const fileLockManagerPort = + await exposeFileLockManager(fileLockManager); const additionalPlayground = await handler.bootPlayground({ From 4910a1da4995bdb406c82ae7dab65a9bdc4fa4ae Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Sun, 23 Nov 2025 19:27:37 -0500 Subject: [PATCH 3/5] Back out breaking native fcntl() use --- .../src/lib/file-lock-manager-for-node.ts | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts b/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts index 5a28a5cfa0..4eaa4e4481 100644 --- a/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts +++ b/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts @@ -243,12 +243,10 @@ export class FileLock { // Therefore, we always request an exclusive lock on Windows. nativeLockingAPI.flockSync(fd, 'ex'); } else { - nativeLockingAPI.fcntlSync( + // TODO: Update locking to obtain native locks for both fcntl() and flock() + nativeLockingAPI.flockSync( fd, - 'setlk', - mode === 'exclusive' - ? nativeLockingAPI.constants.F_WRLCK - : nativeLockingAPI.constants.F_RDLCK + mode === 'exclusive' ? 'ex' : 'sh' ); } @@ -656,20 +654,12 @@ export class FileLock { ); } } else { - const { constants } = this.nativeLock.nativeLockingAPI; const flags = - (requiredNativeLockType === 'exclusive' && - constants.F_WRLCK) || - (requiredNativeLockType === 'shared' && - constants.F_RDLCK) || - constants.F_UNLCK; - - // TODO: Update locking to obtain native locks for both fcntl() and flock() - // operations. On Linux, this is important because fcntl() locks do not conflict - // with flock() locks and vice versa. On macOS, fcntl() and flock() locks can conflict. - this.nativeLock.nativeLockingAPI.fcntlSync( + (requiredNativeLockType === 'exclusive' && 'ex') || + (requiredNativeLockType === 'shared' && 'sh') || + 'un'; + this.nativeLock.nativeLockingAPI.flockSync( this.nativeLock.fd, - 'setlk', flags ); } From e497130d46543bab740f9fc41dc751897360ee35 Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Sun, 23 Nov 2025 19:31:47 -0500 Subject: [PATCH 4/5] Fix type error in FileLockManager tests --- .../php-wasm/node/src/test/file-lock-manager-for-node.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/php-wasm/node/src/test/file-lock-manager-for-node.spec.ts b/packages/php-wasm/node/src/test/file-lock-manager-for-node.spec.ts index ea1fe556a1..fdee063986 100644 --- a/packages/php-wasm/node/src/test/file-lock-manager-for-node.spec.ts +++ b/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}`); }); From 31b009cfca1c4fdb9c3c494d5df22bd27e49fddf Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Sun, 23 Nov 2025 19:47:08 -0500 Subject: [PATCH 5/5] Restore use of non-blocking flock() commands --- .../php-wasm/node/src/lib/file-lock-manager-for-node.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts b/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts index 4eaa4e4481..99b5e9126b 100644 --- a/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts +++ b/packages/php-wasm/node/src/lib/file-lock-manager-for-node.ts @@ -241,12 +241,12 @@ export class FileLock { // 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, 'ex'); + nativeLockingAPI.flockSync(fd, 'exnb'); } else { // TODO: Update locking to obtain native locks for both fcntl() and flock() nativeLockingAPI.flockSync( fd, - mode === 'exclusive' ? 'ex' : 'sh' + mode === 'exclusive' ? 'exnb' : 'shnb' ); } @@ -655,8 +655,8 @@ export class FileLock { } } else { const flags = - (requiredNativeLockType === 'exclusive' && 'ex') || - (requiredNativeLockType === 'shared' && 'sh') || + (requiredNativeLockType === 'exclusive' && 'exnb') || + (requiredNativeLockType === 'shared' && 'shnb') || 'un'; this.nativeLock.nativeLockingAPI.flockSync( this.nativeLock.fd,