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 }})' 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..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 @@ -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,30 @@ 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, '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 { 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}`); }); diff --git a/packages/playground/cli/src/run-cli.ts b/packages/playground/cli/src/run-cli.ts index f1ffbb914b..0b6230d18b 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, @@ -626,22 +625,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;