Fix Bad Gateway and optimize PHP workers#3219
Merged
Conversation
f8d7d38 to
2a7f9b3
Compare
Collaborator
|
Woah all tests are green ❤️ |
5536236 to
efba470
Compare
adamziel
reviewed
Jan 29, 2026
| expect(() => semaphore.acquire()).rejects.toThrow(AcquireTimeoutError); | ||
| }); | ||
|
|
||
| it('should not leave stale resolvers in queue after timeout', async () => { |
efba470 to
3beb8b6
Compare
JanJakes
added a commit
that referenced
this pull request
Feb 2, 2026
This test covers the Bad Gateway issue that was addressed in #3219.
JanJakes
added a commit
that referenced
this pull request
Feb 5, 2026
) ## Motivation for the change, related issues This PR adds a regression test for the Bad Gateway fix implemented in #3219. The test ensures that the `PHPProcessManager` correctly queues requests when all PHP instances are busy and properly reuses instances once they become available. ## Implementation details The test simulates 6 concurrent requests with only 2 PHP instances available, each request taking 50ms to complete. With proper queuing and instance reuse, all 6 requests should complete within the 200ms timeout (3 batches × 50ms = 150ms). The test pre-boots both PHP instances before starting the concurrent requests to eliminate instance spawning time from the timing calculations. ## Testing Instructions (or ideally a Blueprint) CI should be green. The new test can be run using: ``` npx nx run php-wasm-node:test-group-4-jspi --testNamePattern="should correctly queue requests and reuse PHP instances" ``` --------- Co-authored-by: Ashish Kumar <ashfame@users.noreply.github.com>
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.
Motivation for the change, related issues
This PR fixes Bad Gateway (502) errors that occur when multiple concurrent requests are made to the PHP request handler.
Implementation details
The primary PHP instance was not used for enqueued requests. Instead, all enqueued requests needed to wait for the secondary instances, and the primary instance only served new requests directly, never from the queue.
The fix refactors
PHPProcessManagerto a pool-based design where all spawned instances (including primary) are added to an idle pool,acquirePHPInstance()takes from the pool or spawns new ones if needed, and instances are returned to the pool for reuse. All PHP instances should be automatically rotated every 400 requests.Increased instance wait timeout from 5s to 30s: When all PHP instances are busy, new requests wait for one to become available. The previous 5s timeout was likely a bit too short for some scenarios. This timeout limits wait time for an available instance, not PHP execution time.
Semaphore stale resolver bug: When a semaphore acquire timed out, its resolver remained in the queue. Later releases would notify these stale resolvers instead of actual waiting requests. The fix removes timed-out resolvers from the queue.
Testing Instructions (or ideally a Blueprint)
The fix can be verified by running concurrent requests in the browser console:
On
trunk, some of these promises would be rejected. In this branch, all should succeed.