From 92bae60b94e98b4a5e0bb80b76ca989cf0d1e0fe Mon Sep 17 00:00:00 2001 From: Adam Zielinski Date: Tue, 23 Apr 2024 20:24:03 +0200 Subject: [PATCH] PHP: Add a cwd argument to hotSwapPHPRuntime() (#1304) To support [Loopback Request](https://github.com/WordPress/wordpress-playground/pull/1287), we need to decouple BasePHP from the request handler. This PR removes the implicit dependency from hotSwapPHPRuntime, where `requestHandler.documentRoot` was accessed, and replaces it with an explicit `cwd` argument. In the future, that argument will be removed and the PHP constructor will accept a class-level `cwd` property. ## Testing instructions Confirm the CI checks pass. --- .../node/src/test/rotate-php-runtime.spec.ts | 35 ++++++++----------- .../php-wasm/universal/src/lib/base-php.ts | 11 +++--- .../universal/src/lib/rotate-php-runtime.ts | 14 ++++++-- .../remote/src/lib/worker-thread.ts | 1 + 4 files changed, 34 insertions(+), 27 deletions(-) diff --git a/packages/php-wasm/node/src/test/rotate-php-runtime.spec.ts b/packages/php-wasm/node/src/test/rotate-php-runtime.spec.ts index e3b2dd21e8..9040f5e346 100644 --- a/packages/php-wasm/node/src/test/rotate-php-runtime.spec.ts +++ b/packages/php-wasm/node/src/test/rotate-php-runtime.spec.ts @@ -22,11 +22,10 @@ describe('rotatePHPRuntime()', () => { const recreateRuntimeSpy = vitest.fn(recreateRuntime); // Rotate the PHP runtime - const php = new NodePHP(await recreateRuntime(), { - documentRoot: '/test-root', - }); + const php = new NodePHP(await recreateRuntime()); rotatePHPRuntime({ php, + cwd: '/test-root', recreateRuntime: recreateRuntimeSpy, maxRequests: 1000, }); @@ -53,11 +52,10 @@ describe('rotatePHPRuntime()', () => { it('Should recreate the PHP runtime after maxRequests', async () => { const recreateRuntimeSpy = vitest.fn(recreateRuntime); - const php = new NodePHP(await recreateRuntimeSpy(), { - documentRoot: '/test-root', - }); + const php = new NodePHP(await recreateRuntimeSpy()); rotatePHPRuntime({ php, + cwd: '/test-root', recreateRuntime: recreateRuntimeSpy, maxRequests: 1, }); @@ -68,11 +66,10 @@ describe('rotatePHPRuntime()', () => { it('Should stop rotating after the cleanup handler is called', async () => { const recreateRuntimeSpy = vitest.fn(recreateRuntime); - const php = new NodePHP(await recreateRuntimeSpy(), { - documentRoot: '/test-root', - }); + const php = new NodePHP(await recreateRuntimeSpy()); const cleanup = rotatePHPRuntime({ php, + cwd: '/test-root', recreateRuntime: recreateRuntimeSpy, maxRequests: 1, }); @@ -98,11 +95,10 @@ describe('rotatePHPRuntime()', () => { } return recreateRuntime('8.3'); }); - const php = new NodePHP(await recreateRuntimeSpy(), { - documentRoot: '/test-root', - }); + const php = new NodePHP(await recreateRuntimeSpy()); rotatePHPRuntime({ php, + cwd: '/test-root', recreateRuntime: recreateRuntimeSpy, maxRequests: 1, }); @@ -121,11 +117,10 @@ describe('rotatePHPRuntime()', () => { }, 30_000); it('Should preserve the custom SAPI name', async () => { - const php = new NodePHP(await recreateRuntime(), { - documentRoot: '/test-root', - }); + const php = new NodePHP(await recreateRuntime()); rotatePHPRuntime({ php, + cwd: '/test-root', recreateRuntime, maxRequests: 1, }); @@ -140,11 +135,10 @@ describe('rotatePHPRuntime()', () => { }); it('Should preserve the MEMFS files', async () => { - const php = new NodePHP(await recreateRuntime(), { - documentRoot: '/test-root', - }); + const php = new NodePHP(await recreateRuntime()); rotatePHPRuntime({ php, + cwd: '/test-root', recreateRuntime, maxRequests: 1, }); @@ -165,11 +159,10 @@ describe('rotatePHPRuntime()', () => { }, 30_000); it('Should not overwrite the NODEFS files', async () => { - const php = new NodePHP(await recreateRuntime(), { - documentRoot: '/test-root', - }); + const php = new NodePHP(await recreateRuntime()); rotatePHPRuntime({ php, + cwd: '/test-root', recreateRuntime, maxRequests: 1, }); diff --git a/packages/php-wasm/universal/src/lib/base-php.ts b/packages/php-wasm/universal/src/lib/base-php.ts index 5bfd0315a8..6f5b90821a 100644 --- a/packages/php-wasm/universal/src/lib/base-php.ts +++ b/packages/php-wasm/universal/src/lib/base-php.ts @@ -825,8 +825,12 @@ export abstract class BasePHP implements IsomorphicLocalPHP { * interrupting the operations of this PHP instance. * * @param runtime + * @param cwd. Internal, the VFS path to recreate in the new runtime. + * This arg is temporary and will be removed once BasePHP + * is fully decoupled from the request handler and + * accepts a constructor-level cwd argument. */ - hotSwapPHPRuntime(runtime: number) { + hotSwapPHPRuntime(runtime: number, cwd?: string) { // Once we secure the lock and have the new runtime ready, // the rest of the swap handler is synchronous to make sure // no other operations acts on the old runtime or FS. @@ -858,9 +862,8 @@ export abstract class BasePHP implements IsomorphicLocalPHP { } // Copy the MEMFS directory structure from the old FS to the new one - if (this.requestHandler) { - const docroot = this.documentRoot; - copyFS(oldFS, this[__private__dont__use].FS, docroot); + if (cwd) { + copyFS(oldFS, this[__private__dont__use].FS, cwd); } } diff --git a/packages/php-wasm/universal/src/lib/rotate-php-runtime.ts b/packages/php-wasm/universal/src/lib/rotate-php-runtime.ts index 17542ba824..b19f7a0943 100644 --- a/packages/php-wasm/universal/src/lib/rotate-php-runtime.ts +++ b/packages/php-wasm/universal/src/lib/rotate-php-runtime.ts @@ -2,6 +2,7 @@ import { BasePHP } from './base-php'; export interface RotateOptions { php: T; + cwd: string; recreateRuntime: () => Promise | number; maxRequests: number; } @@ -24,8 +25,17 @@ export interface RotateOptions { */ export function rotatePHPRuntime({ php, + cwd, recreateRuntime, - maxRequests, + /* + * 400 is an arbitrary number that should trigger a rotation + * way before the memory gets too fragmented. If it doesn't, + * let's explore: + * * Rotating based on an actual memory usage and + * fragmentation. + * * Resetting HEAP to its initial value. + */ + maxRequests = 400, }: RotateOptions) { let handledCalls = 0; async function rotateRuntime() { @@ -36,7 +46,7 @@ export function rotatePHPRuntime({ const release = await php.semaphore.acquire(); try { - php.hotSwapPHPRuntime(await recreateRuntime()); + php.hotSwapPHPRuntime(await recreateRuntime(), cwd); } finally { release(); } diff --git a/packages/playground/remote/src/lib/worker-thread.ts b/packages/playground/remote/src/lib/worker-thread.ts index 5ccefb5076..1a001ca171 100644 --- a/packages/playground/remote/src/lib/worker-thread.ts +++ b/packages/playground/remote/src/lib/worker-thread.ts @@ -164,6 +164,7 @@ const recreateRuntime = async () => { // @see https://github.com/WordPress/wordpress-playground/pull/990 for more context rotatePHPRuntime({ php, + cwd: DOCROOT, recreateRuntime, // 400 is an arbitrary number that should trigger a rotation // way before the memory gets too fragmented. If the memory