Repository navigation
[PHP Process Manager] Support acquiring a non-primary PHP instance - #2274
Merged
Merged
Conversation
adamziel
force-pushed
the
push-mvwwlltywvzv
branch
from
June 16, 2025 09:35
1fb276c to
7626e8f
Compare
adamziel
force-pushed
the
push-mvwwlltywvzv
branch
from
June 16, 2025 09:57
7626e8f to
71b5342
Compare
adamziel
force-pushed
the
push-wqrnpnvxmzpm
branch
2 times, most recently
from
June 16, 2025 11:07
fc4e46a to
dbb07c3
Compare
adamziel
force-pushed
the
push-mvwwlltywvzv
branch
3 times, most recently
from
June 16, 2025 11:23
4ebeb9a to
35a47f8
Compare
adamziel
marked this pull request as ready for review
June 16, 2025 11:23
/** * 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
force-pushed
the
push-mvwwlltywvzv
branch
from
June 16, 2025 14:15
35a47f8 to
10480ec
Compare
adamziel
force-pushed
the
push-mvwwlltywvzv
branch
from
June 16, 2025 15:29
936dbba to
898b549
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
considerPrimaryoption toprocessManager.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.considerPrimarydefaults totrueso we must be very careful about how we acquire a PHP instance before running.cli(). Ideally it would default tofalse, making.cli()calls safe by default. Unfortunately, that brakes a bunch of tests, especially aroundmaxPhpInstanceand memory limits. Two possible solutions are:In addition, this PR fixes a race condition in
spawn(). Spawning the primary PHP instance was racing when called multiple times. By introducing aprimaryPhpPromisecache, 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.