Skip to content

Commit fdb90bf

Browse files
ondrejmirtesclaude
andcommitted
Lock Composer autoload around phar:// reads in forked workers
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>
1 parent a4d403a commit fdb90bf

3 files changed

Lines changed: 112 additions & 1 deletion

File tree

src/Command/FixerApplication.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use PHPStan\Internal\DirectoryCreatorException;
2626
use PHPStan\Internal\HttpClientFactory;
2727
use PHPStan\Parallel\ForkParallelChecker;
28+
use PHPStan\Parallel\PharAutoloaderLock;
2829
use PHPStan\PhpDoc\StubFilesProvider;
2930
use PHPStan\Process\ForkedProcessPromise;
3031
use PHPStan\Process\ProcessCanceledException;
@@ -100,6 +101,7 @@ public function __construct(
100101
private HttpClientFactory $httpClientFactory,
101102
private ForkParallelChecker $forkParallelChecker,
102103
private FixerWorkerRunner $fixerWorkerRunner,
104+
private PharAutoloaderLock $pharAutoloaderLock,
103105
)
104106
{
105107
}
@@ -457,8 +459,13 @@ private function analyse(
457459
});
458460
});
459461

462+
$useFork = $this->forkParallelChecker->isSupported();
463+
if ($useFork) {
464+
$this->pharAutoloaderLock->install();
465+
}
466+
460467
$process = $this->createProcessPromise(
461-
$this->forkParallelChecker->isSupported(),
468+
$useFork,
462469
$loop,
463470
$server,
464471
$mainScript,

src/Parallel/ParallelAnalyser.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public function __construct(
5454
private int $decoderBufferSize,
5555
private ForkParallelChecker $forkParallelChecker,
5656
private WorkerRunner $workerRunner,
57+
private PharAutoloaderLock $pharAutoloaderLock,
5758
)
5859
{
5960
$this->processTimeout = max($processTimeout, self::DEFAULT_TIMEOUT);
@@ -175,6 +176,9 @@ public function analyse(
175176
};
176177

177178
$useFork = $this->forkParallelChecker->isSupported();
179+
if ($useFork) {
180+
$this->pharAutoloaderLock->install();
181+
}
178182

179183
for ($i = 0; $i < $numberOfProcesses; $i++) {
180184
if (count($jobs) === 0) {
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Parallel;
4+
5+
use Composer\Autoload\ClassLoader;
6+
use Phar;
7+
use PHPStan\DependencyInjection\AutowiredService;
8+
use function fclose;
9+
use function flock;
10+
use function fopen;
11+
use function getmypid;
12+
use function register_shutdown_function;
13+
use function spl_autoload_register;
14+
use function sys_get_temp_dir;
15+
use function touch;
16+
use function uniqid;
17+
use function unlink;
18+
use const LOCK_EX;
19+
use const LOCK_UN;
20+
21+
/**
22+
* Serialises Composer autoload reads from a shared phar fd so concurrent
23+
* forked workers can't race on its seek cursor.
24+
*
25+
* When PHPStan is run from a .phar (composer-distributed install), PHP's
26+
* built-in phar:// stream wrapper caches a single fd internally; after
27+
* pcntl_fork() that fd's open file description (and its seek cursor) is
28+
* shared between parent and every forked child, so concurrent lazy class
29+
* loads across workers can interleave and read garbage offsets — surfacing
30+
* as spurious parse errors against phar-internal files.
31+
*
32+
* The minimal-surgery fix here: wrap every registered Composer ClassLoader
33+
* so that loadClass() acquires an exclusive flock on a tmp file before its
34+
* `include`, and releases it after. Two workers can never be inside
35+
* `include 'phar://…'` at the same time, so the cursor can't be moved out
36+
* from under either of them.
37+
*
38+
* Cost model: per-class load takes one flock pair. A worker contends with
39+
* siblings only during its initial "loading wave" — the few hundred classes
40+
* it touches once. After that its symbol table is populated and the lock
41+
* is never taken again, so workers run fully parallel for the rest of the
42+
* analysis.
43+
*
44+
* Covers only autoload. Non-class phar reads — file_get_contents('phar://…')
45+
* etc. — still go through the unlocked built-in wrapper; in practice those
46+
* happen during boot, which is pre-fork, so they don't race. If a lazy
47+
* non-class phar read does fire post-fork, the alternative extract-and-
48+
* reroute approach (see #5669) is the comprehensive fix.
49+
*
50+
* No-op when not running inside a phar; called by ParallelAnalyser and
51+
* FixerApplication right before they fork. Idempotent.
52+
*/
53+
#[AutowiredService]
54+
final class PharAutoloaderLock
55+
{
56+
57+
private bool $installed = false;
58+
59+
public function install(): void
60+
{
61+
if ($this->installed) {
62+
return;
63+
}
64+
$this->installed = true;
65+
66+
if (Phar::running(false) === '') {
67+
return;
68+
}
69+
70+
$lockPath = sys_get_temp_dir() . '/phpstan-fork-phar-lock-' . getmypid() . '-' . uniqid();
71+
touch($lockPath);
72+
73+
foreach (ClassLoader::getRegisteredLoaders() as $loader) {
74+
$loader->unregister();
75+
spl_autoload_register(static function (string $class) use ($loader, $lockPath): void {
76+
$fh = fopen($lockPath, 'r');
77+
if ($fh === false) {
78+
$loader->loadClass($class);
79+
return;
80+
}
81+
flock($fh, LOCK_EX);
82+
try {
83+
$loader->loadClass($class);
84+
} finally {
85+
flock($fh, LOCK_UN);
86+
fclose($fh);
87+
}
88+
});
89+
}
90+
91+
$parentPid = getmypid();
92+
register_shutdown_function(static function () use ($parentPid, $lockPath): void {
93+
if (getmypid() !== $parentPid) {
94+
return;
95+
}
96+
@unlink($lockPath);
97+
});
98+
}
99+
100+
}

0 commit comments

Comments
 (0)