Lock Composer autoload around phar:// reads in forked workers#5670
Open
ondrejmirtes wants to merge 3 commits into
Open
Lock Composer autoload around phar:// reads in forked workers#5670ondrejmirtes wants to merge 3 commits into
ondrejmirtes wants to merge 3 commits into
Conversation
Alternative approach to making the experimental pcntl_fork() worker path work when PHPStan is run from a .phar. Where #5669 extracts the phar before forking and reroutes every phar:// read to disk, this takes the opposite tack: leave PHP's built-in phar:// wrapper in place but serialise the racy operation. PHP's built-in phar:// stream wrapper caches a single fd for the running .phar; after pcntl_fork() that fd's open file description (and its seek cursor) is shared between parent and every forked child, so concurrent lazy class loads across workers can interleave and read garbage offsets — surfacing as spurious parse errors against phar-internal files. PharAutoloaderLock::install() wraps every registered Composer ClassLoader so loadClass() acquires an exclusive flock on a tmp file before its `include` and releases it after. Two workers can never hold the lock simultaneously, so they can never be inside `include 'phar://…'` simultaneously, so the cursor can't be moved out from under either of them. Cost: per-class load takes one flock pair. Each worker contends with siblings only during its initial "loading wave" — a few hundred classes touched once. After its symbol table is populated the lock is never taken again and workers run fully parallel for the rest of the analysis. Covers only class autoload. Non-class phar reads (file_get_contents('phar://…'), stat, dir iteration) still go through the unlocked built-in wrapper; in practice those happen during boot, which is pre-fork, so they don't race. ParallelAnalyser and FixerApplication call the (idempotent) install() once they have decided to take the fork path. No-op when not running inside a phar. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… reference Referencing Composer\Autoload\ClassLoader makes php-scoper rewrite it to the prefixed _PHPStan_xxx\Composer\Autoload\ClassLoader form when building the phar, but Composer's autoloader is exempted from prefixing by the scoper patcher whitelist in compiler/build/scoper.inc.php (it lives at the unscoped name at runtime). So the prefixed reference resolves to a non-existent class and PharAutoloaderLock::install() fatals on first call. Switch to spl_autoload_functions() / spl_autoload_register / spl_autoload_unregister, working with the registered callables directly. No reference to Composer's namespace, no dependency on the scoper patcher's whitelist, no Composer-version assumptions (getRegisteredLoaders() is 2.0+). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ted autoload PHP class declaration can trigger a nested autoload inside the declaration itself — `class Foo extends Bar` autoloads Bar mid-bind. With each call doing its own `fopen` + `flock(LOCK_EX)`, the nested call opens a separate OFD against the same lockfile and tries to acquire LOCK_EX while the outer call holds it; flock is per-OFD on BSD/macOS, so the same process self-deadlocks. Symptom: 0/N files progress and every worker stuck inside `zif_flock` according to sample(1). The race that actually matters is cross-process — within one process, libphar reads are sequential by construction, so the nested call doesn't need a second lock. Track depth per-process: outermost call takes the flock, nested ones (depth > 0) skip straight to the delegate. Cross-process exclusion is preserved because the static counter is per-process (fork COWs its own copy). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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
1 participant
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.
Alternative to #5669. Same problem (PHP's built-in
phar://wrappercaches one fd, after
pcntl_fork()parent and forked children shareits OFD, concurrent class loads race on the seek cursor and produce
spurious parse errors). Different fix: wrap every registered Composer
ClassLoadersoloadClass()takes an exclusiveflockon a tmp filebefore its
includeand releases it after. Two workers can never beinside
include 'phar://…'simultaneously, so the cursor can't bemoved out from under either of them.
Cost
One
flockpair per autoload. Each worker only contends with siblingsduring its initial loading wave — a few hundred classes touched once.
After its symbol table is populated the lock is never taken again and
workers run fully parallel for the rest of the analysis.
Scope
Covers only class autoload. Non-class
phar://reads(
file_get_contents, stat, dir iteration) still go through theunlocked built-in wrapper. In practice those happen during boot, which
is pre-fork, so they don't race. If a lazy non-class phar read does
fire post-fork, #5669 (extract-and-reroute) is the comprehensive fix.
Wiring
ParallelAnalyserandFixerApplicationcall the (idempotent)install()once they have decided to take the fork path. No-op whennot running inside a phar. Cleanup hook unlinks the lock tmpfile on
parent shutdown.
🤖 Generated with Claude Code