Fix agent startup race across PHP runtimes - #447
Open
teta2k wants to merge 9 commits into
Open
Conversation
teta2k
marked this pull request as ready for review
August 14, 2026 07:48
tomaisthorpe
left a comment
Contributor
There was a problem hiding this comment.
I think there's potential issue here if a worker fails during startup, something like the following:
- N processes start
- One process wins the lock, the other processes continue as another process has the lock, they report success
- During startup, the winning worker fails.
- Lock is released, but it's not noticed, and isn't reattempted.
Collaborator
Author
Thanks. I updated the launcher to wait for the shared socket instead of treating a held lock as success. If the winning worker dies before the socket is ready, its launcher reaps it and retries while the other launchers keep waiting. I tested this by killing the winning worker before socket creation, and it recovered with exactly one worker in 100/100 runs. |
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.
Summary
Fix a long-standing race in Aikido Agent startup.
The PHP extension now starts a short-lived launcher with
posix_spawn(). A worker acquires a kernel-enforced exclusive lock before initialization and owns the Unix socket and gRPC server for its lifetime. The launcher waits until that shared socket is ready and retries if its worker fails during startup.The issue was discovered while testing 1.5.18, but the affected startup design predates that release.
Problem
Previously, every PHP runtime initialized the Agent using several separate operations:
/proc.fork()anddaemon().These operations were not performed as one indivisible decision: another process could start or stop an Agent between the check and the following action. Concurrent PHP initialization could therefore allow several processes to make conflicting startup and cleanup decisions.
There was an additional problem in FrankenPHP: it runs PHP in a multithreaded process. Calling
fork()ordaemon()there creates a child containing only the calling thread while retaining state inherited from the other threads. The result depends on timing, which explains why the missing socket occurred intermittently. FrankenPHP internalsRequest-local attack detection does not depend on this socket, so SQL injection blocking could continue working while metadata RPC calls repeatedly failed.
New architecture
All SAPIs use this same path:
fork()/daemon().FrankenPHP is the most direct production case because even one server is multithreaded, making the previous
fork()/daemon()path unsafe. PHP-FPM and Apache still need correct singleton behavior when independent server startups overlap during deployments, reloads, or supervisor retries. The ten-process stress test deliberately forces that worst-case overlap to verify the singleton guarantee; it does not model normal worker startup or represent a production failure rate.The launcher probes the runtime-directory lock before starting a candidate. The worker then opens and locks that directory itself. If candidates start at the same time, only the worker that acquires the lock initializes; the others exit and their launchers continue waiting for the shared socket.
The lock is held on the existing
/run/aikido-<version>directory, so no additional singleton file or fixed file-descriptor handoff is needed. A worker that fails before creating the socket releases the lock automatically. Its launcher reaps the failed process and starts a replacement.PHP waits for and reaps only the short-lived launcher. The long-lived worker is detached and adopted by the container init process or subreaper.
What changed
/procchecks with one atomic directory lock.fork()anddaemon()from the PHP host process.Stress-test method
The same concurrent-start test was run separately with PHP 7.2 NTS, PHP 8.2 ZTS, PHP 8.5 NTS, and PHP 8.5 ZTS.
The purpose was to verify the singleton guarantee: all PHP processes using the same Aikido version in one container must share exactly one Agent worker and one Unix socket. Multiple workers would compete for the same socket and reproduce the startup race this PR fixes.
In each trial the test:
aikido-agent.sock;--agent-workerprocess and a Unix socket;The PHP built-in server (
php -S) was used only as a small test driver that loads the extension in ten independent PHP processes and keeps them alive. Each launcher stopped itself immediately before executing PHP; oneSIGCONTthen released all ten launchers together:The barrier makes all ten extension initializations begin at the same time instead of depending on shell and scheduler timing. This isolates the cross-process Agent startup race; it is not presented as a production SAPI test. Actual FrankenPHP classic and worker processes were tested separately. Apache and PHP-FPM were exercised by the normal integration-test matrix, not by this 100-trial stress harness.
Local stress results
Each value is successful trials out of 100. A trial passed only when all ten synchronized
php -Sprocesses converged on exactly one live Agent worker and the Unix socket existed. A duplicate or missing Agent, or a missing socket, counted as a failure. These deliberately synchronized stress results are not production failure rates.Actual FrankenPHP classic and worker servers were also tested separately with PHP 8.2 and PHP 8.5. All four patched profiles completed 100/100 startup trials.
A separate test killed the selected worker before it created the socket. The waiting launchers reaped the failed process and recovered with one replacement worker in 100/100 trials.
Validation
go vetpass withCGO_ENABLED=0.git diff --checkpasses.