From 6c2a7ae1b5647024c175894133038b66a42de455 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 21 Oct 2026 09:08:09 -0400 Subject: [PATCH 1/2] Give each PHP worker a distinct port Worker ports were picked one at a time, each probe closed before the next, so the OS could return a port it had just released. Two workers then got the same port and one failed to bind it, stopping the site from starting. Keep every probe bound until all ports are chosen. --- .../lib/native-php/tests/worker-ports.test.ts | 68 +++++++++++++++++++ apps/cli/lib/native-php/worker-ports.ts | 47 +++++++++++++ apps/cli/php-server-child.ts | 24 +------ 3 files changed, 117 insertions(+), 22 deletions(-) create mode 100644 apps/cli/lib/native-php/tests/worker-ports.test.ts create mode 100644 apps/cli/lib/native-php/worker-ports.ts diff --git a/apps/cli/lib/native-php/tests/worker-ports.test.ts b/apps/cli/lib/native-php/tests/worker-ports.test.ts new file mode 100644 index 0000000000..9ab76dd6de --- /dev/null +++ b/apps/cli/lib/native-php/tests/worker-ports.test.ts @@ -0,0 +1,68 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it, vi } from 'vitest'; +import { getAvailablePorts } from '../worker-ports'; + +// An OS that hands out the lowest free port, so it reuses a port the moment it is +// released. Real kernels can do the same (Linux picks ephemeral ports at random, +// and other processes take ports in between), which is what this guards against. +const { boundPorts } = vi.hoisted( () => ( { boundPorts: new Set< number >() } ) ); + +vi.mock( 'node:net', async () => { + const { EventEmitter } = await import( 'node:events' ); + + class FakeServer extends EventEmitter { + private port: number | undefined; + listening = false; + + listen( _port: number, _host: string, onListening: () => void ) { + let port = 50_000; + while ( boundPorts.has( port ) ) { + port++; + } + boundPorts.add( port ); + this.port = port; + this.listening = true; + setImmediate( onListening ); + return this; + } + + address() { + return this.port === undefined + ? null + : { port: this.port, address: '127.0.0.1', family: 'IPv4' }; + } + + close( onClose?: () => void ) { + if ( this.port !== undefined ) { + boundPorts.delete( this.port ); + } + this.listening = false; + setImmediate( () => onClose?.() ); + return this; + } + + unref() { + return this; + } + } + + const net = { createServer: () => new FakeServer() }; + return { default: net, ...net }; +} ); + +describe( 'getAvailablePorts', () => { + it( 'returns distinct ports even when the OS reuses released ports', async () => { + const ports = await getAvailablePorts( 4 ); + + expect( ports ).toHaveLength( 4 ); + expect( new Set( ports ).size ).toBe( 4 ); + } ); + + it( 'releases every port before returning, so the workers can bind them', async () => { + await getAvailablePorts( 4 ); + + expect( boundPorts.size ).toBe( 0 ); + } ); +} ); diff --git a/apps/cli/lib/native-php/worker-ports.ts b/apps/cli/lib/native-php/worker-ports.ts new file mode 100644 index 0000000000..fd8e22ac3c --- /dev/null +++ b/apps/cli/lib/native-php/worker-ports.ts @@ -0,0 +1,47 @@ +import net from 'node:net'; + +function listenOnFreePort( server: net.Server ): Promise< number > { + return new Promise( ( resolve, reject ) => { + server.once( 'error', reject ); + server.listen( 0, '127.0.0.1', () => { + const address = server.address(); + if ( ! address || typeof address === 'string' ) { + reject( new Error( 'Could not allocate a PHP worker port' ) ); + return; + } + resolve( address.port ); + } ); + } ); +} + +/** + * Returns `count` distinct free ports for the PHP workers. Each probe server stays + * bound until every port is chosen, so the OS cannot hand a port it just released + * to the next probe. + */ +export async function getAvailablePorts( count: number ): Promise< number[] > { + const servers: net.Server[] = []; + try { + const ports: number[] = []; + for ( let index = 0; index < count; index++ ) { + const server = net.createServer(); + server.unref(); + servers.push( server ); + ports.push( await listenOnFreePort( server ) ); + } + return ports; + } finally { + await Promise.all( + servers.map( + ( server ) => + new Promise< void >( ( resolve ) => { + if ( ! server.listening ) { + resolve(); + return; + } + server.close( () => resolve() ); + } ) + ) + ); + } +} diff --git a/apps/cli/php-server-child.ts b/apps/cli/php-server-child.ts index 66a39bc0c0..bbd06d7413 100644 --- a/apps/cli/php-server-child.ts +++ b/apps/cli/php-server-child.ts @@ -8,7 +8,6 @@ import fs from 'node:fs'; import http from 'node:http'; -import net from 'node:net'; import os from 'node:os'; import path from 'node:path'; import { writeStudioMuPlugins } from '@studio/common/lib/mu-plugins'; @@ -43,6 +42,7 @@ import { installWordPress, writeSiteUrlPrependFile, } from './lib/native-php/site-setup'; +import { getAvailablePorts } from './lib/native-php/worker-ports'; import { SymlinkWatcher, collectSymlinkAllowlistEntries } from './lib/symlinks'; import type { ChildProcess } from 'node:child_process'; @@ -173,23 +173,6 @@ function pickPhpWorker( req: http.IncomingMessage ): { index: number; port: numb return { index: bestIndex, port: phpWorkerPorts[ bestIndex ] }; } -async function getAvailablePort(): Promise< number > { - return await new Promise< number >( ( resolve, reject ) => { - const server = net.createServer(); - server.unref(); - server.once( 'error', reject ); - server.listen( 0, '127.0.0.1', () => { - const address = server.address(); - if ( ! address || typeof address === 'string' ) { - server.close( () => reject( new Error( 'Could not allocate a PHP worker port' ) ) ); - return; - } - const port = address.port; - server.close( () => resolve( port ) ); - } ); - } ); -} - async function waitForServerReady( url: string, signal?: AbortSignal ): Promise< void > { const pollIntervalMs = 50; const timeoutMs = 30_000; @@ -577,10 +560,7 @@ async function doStartServer( const phpMyAdminWpEnvPath = await writeNativePhpMyAdminWpEnv( config ); const siteUrl = config.absoluteUrl || `http://localhost:${ config.port }`; const autoPrependFile = writeSiteUrlPrependFile( siteUrl, config.autoPrependFile ); - const workerPorts: number[] = []; - for ( let index = 0; index < NATIVE_PHP_WORKER_POOL_SIZE; index++ ) { - workerPorts.push( await getAvailablePort() ); - } + const workerPorts = await getAvailablePorts( NATIVE_PHP_WORKER_POOL_SIZE ); phpWorkerPorts = workerPorts; phpWorkerRequestTracker = new PhpWorkerRequestTracker( workerPorts.length ); From 06c6f929d0af429e6da343e72d9b23c65e520460 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 27 Oct 2026 09:21:33 -0400 Subject: [PATCH 2/2] Trim worker port allocation and its test --- .../lib/native-php/tests/worker-ports.test.ts | 68 ++++++------------- apps/cli/lib/native-php/worker-ports.ts | 43 +++--------- 2 files changed, 29 insertions(+), 82 deletions(-) diff --git a/apps/cli/lib/native-php/tests/worker-ports.test.ts b/apps/cli/lib/native-php/tests/worker-ports.test.ts index 9ab76dd6de..c30fd14ef2 100644 --- a/apps/cli/lib/native-php/tests/worker-ports.test.ts +++ b/apps/cli/lib/native-php/tests/worker-ports.test.ts @@ -1,68 +1,40 @@ /** * @vitest-environment node */ -import { describe, expect, it, vi } from 'vitest'; +import { expect, it, vi } from 'vitest'; import { getAvailablePorts } from '../worker-ports'; -// An OS that hands out the lowest free port, so it reuses a port the moment it is -// released. Real kernels can do the same (Linux picks ephemeral ports at random, -// and other processes take ports in between), which is what this guards against. +// An OS that always hands out the lowest free port, so a released port comes straight back. +// Linux does this at random (about 1 in 1,000 four-port pools on a real kernel). const { boundPorts } = vi.hoisted( () => ( { boundPorts: new Set< number >() } ) ); -vi.mock( 'node:net', async () => { - const { EventEmitter } = await import( 'node:events' ); - - class FakeServer extends EventEmitter { - private port: number | undefined; - listening = false; - +vi.mock( 'node:net', () => { + class FakeServer { + port = 0; + once() {} + unref() { + return this; + } listen( _port: number, _host: string, onListening: () => void ) { - let port = 50_000; - while ( boundPorts.has( port ) ) { - port++; - } - boundPorts.add( port ); - this.port = port; - this.listening = true; + for ( this.port = 50_000; boundPorts.has( this.port ); this.port++ ); + boundPorts.add( this.port ); setImmediate( onListening ); - return this; } - address() { - return this.port === undefined - ? null - : { port: this.port, address: '127.0.0.1', family: 'IPv4' }; - } - - close( onClose?: () => void ) { - if ( this.port !== undefined ) { - boundPorts.delete( this.port ); - } - this.listening = false; - setImmediate( () => onClose?.() ); - return this; + return { port: this.port }; } - - unref() { - return this; + close( onClose: () => void ) { + boundPorts.delete( this.port ); + setImmediate( onClose ); } } - const net = { createServer: () => new FakeServer() }; return { default: net, ...net }; } ); -describe( 'getAvailablePorts', () => { - it( 'returns distinct ports even when the OS reuses released ports', async () => { - const ports = await getAvailablePorts( 4 ); - - expect( ports ).toHaveLength( 4 ); - expect( new Set( ports ).size ).toBe( 4 ); - } ); - - it( 'releases every port before returning, so the workers can bind them', async () => { - await getAvailablePorts( 4 ); +it( 'returns distinct ports and releases them all for the workers to bind', async () => { + const ports = await getAvailablePorts( 4 ); - expect( boundPorts.size ).toBe( 0 ); - } ); + expect( new Set( ports ).size ).toBe( 4 ); + expect( boundPorts.size ).toBe( 0 ); } ); diff --git a/apps/cli/lib/native-php/worker-ports.ts b/apps/cli/lib/native-php/worker-ports.ts index fd8e22ac3c..5caa45b6fe 100644 --- a/apps/cli/lib/native-php/worker-ports.ts +++ b/apps/cli/lib/native-php/worker-ports.ts @@ -1,47 +1,22 @@ import net from 'node:net'; -function listenOnFreePort( server: net.Server ): Promise< number > { - return new Promise( ( resolve, reject ) => { - server.once( 'error', reject ); - server.listen( 0, '127.0.0.1', () => { - const address = server.address(); - if ( ! address || typeof address === 'string' ) { - reject( new Error( 'Could not allocate a PHP worker port' ) ); - return; - } - resolve( address.port ); - } ); - } ); -} - -/** - * Returns `count` distinct free ports for the PHP workers. Each probe server stays - * bound until every port is chosen, so the OS cannot hand a port it just released - * to the next probe. - */ +// Returns `count` distinct free ports for the PHP workers. Every probe stays bound until all the +// ports are chosen, so the OS can't hand a port that was just released to the next probe. export async function getAvailablePorts( count: number ): Promise< number[] > { const servers: net.Server[] = []; try { - const ports: number[] = []; for ( let index = 0; index < count; index++ ) { - const server = net.createServer(); - server.unref(); + const server = net.createServer().unref(); servers.push( server ); - ports.push( await listenOnFreePort( server ) ); + await new Promise< void >( ( resolve, reject ) => { + server.once( 'error', reject ); + server.listen( 0, '127.0.0.1', resolve ); + } ); } - return ports; + return servers.map( ( server ) => ( server.address() as net.AddressInfo ).port ); } finally { await Promise.all( - servers.map( - ( server ) => - new Promise< void >( ( resolve ) => { - if ( ! server.listening ) { - resolve(); - return; - } - server.close( () => resolve() ); - } ) - ) + servers.map( ( server ) => new Promise( ( resolve ) => server.close( resolve ) ) ) ); } }