Skip to content

[PHP Process Manager] Support acquiring a non-primary PHP instance - #2274

Merged
adamziel merged 7 commits into
trunkfrom
push-mvwwlltywvzv
Jun 16, 2025
Merged

adamziel merged 7 commits into
trunkfrom
push-mvwwlltywvzv

Conversation

@adamziel

@adamziel adamziel commented Jun 15, 2025 •

Copy link
Copy Markdown
Collaborator

Adds a considerPrimary option to processManager.acquirePHPInstance.

PHP’s CLI SAPI always exits the process at the end of a run, so using the primary instance for ad-hoc runs breaks its long-lived reuse. By default, acquirePHPInstance() will now ignore the primary slot unless you explicitly opt in. This lets us safely use one-off children by default and tap into the primary only when you really need a persistent session.

considerPrimary defaults to true so we must be very careful about how we acquire a PHP instance before running .cli(). Ideally it would default to false, making .cli() calls safe by default. Unfortunately, that brakes a bunch of tests, especially around maxPhpInstance and memory limits. Two possible solutions are:

  • Address each failure, which I don't think is worth the time investment
  • Decouple PHP from the filesystem #2269 – I think that's the way to go. We could then remove the entire concept of a "primary PHP instance".

In addition, this PR fixes a race condition in spawn(). Spawning the primary PHP instance was racing when called multiple times. By introducing a primaryPhpPromise cache, we ensure that only one spawn happens at a time and any concurrent callers await the same promise. Once resolved, we clear the promise so future calls will still retrieve the already-spawned instance.

Testing instructions

Rely on CI checks.

@adamziel
adamziel force-pushed the push-mvwwlltywvzv branch from 1fb276c to 7626e8f Compare June 16, 2025 09:35
@adamziel
adamziel changed the base branch from trunk to push-wqrnpnvxmzpm June 16, 2025 09:35
@adamziel
adamziel force-pushed the push-mvwwlltywvzv branch from 7626e8f to 71b5342 Compare June 16, 2025 09:57
@adamziel
adamziel force-pushed the push-wqrnpnvxmzpm branch 2 times, most recently from fc4e46a to dbb07c3 Compare June 16, 2025 11:07
@adamziel
adamziel changed the base branch from push-wqrnpnvxmzpm to trunk June 16, 2025 11:07
@adamziel
adamziel force-pushed the push-mvwwlltywvzv branch 3 times, most recently from 4ebeb9a to 35a47f8 Compare June 16, 2025 11:23
@adamziel
adamziel marked this pull request as ready for review June 16, 2025 11:23
adamziel added 4 commits June 16, 2025 16:12
	/**
	 * Get a PHP instance.
	 *
	 * It could be either the primary PHP instance, an idle disposable PHP
	 * instance, or a newly spawned PHP instance – depending on the resource
	 * availability.
	 *
	 * @PARAM considerPrimary - Whether to consider the primary PHP instance.
	 *                          It matters because PHP.cli() sets the SAPI to CLI and
	 *                          kills the entire process after it finishes running,
	 *                          making the primary PHP instance non-reusable for
	 *                          subsequent .run() calls. This is fine for one-off
	 *                          child PHP instances, but not for the primary PHP
	 *                          that's meant to continue working for the entire duration
	 *                          of the ProcessManager lifetime. Therefore, we don't
	 *                          consider the primary PHP instance by default unless
	 *                          the caller explicitly requests it.
	 *
	 * @throws {MaxPhpInstancesError} when the maximum number of PHP instances is reached
	 *                                and the waiting timeout is exceeded.
	 */
@adamziel
adamziel force-pushed the push-mvwwlltywvzv branch from 35a47f8 to 10480ec Compare June 16, 2025 14:15
@adamziel
adamziel force-pushed the push-mvwwlltywvzv branch from 936dbba to 898b549 Compare June 16, 2025 15:29
@adamziel
adamziel merged commit ec9ba63 into trunk Jun 16, 2025
@adamziel
adamziel deleted the push-mvwwlltywvzv branch June 16, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Inbox

Development

Successfully merging this pull request may close these issues.

1 participant