Skip to content

Commit 119fcc3

Browse files
committed
Test that a forked worker does not delete the composed ini
A PHP process spawned during the run reads its configuration from the composed ini through PHPRC. Once a forked worker deletes it, the spawned process loads neither the ini nor the scan dir, and misses every extension loaded from them. Amp-Thread-ID: https://ampcode.com/threads/T-01a17e3b-9bb8-77b1-a5c1-722d8ade601b
1 parent d679562 commit 119fcc3

5 files changed

Lines changed: 165 additions & 0 deletions

File tree

‎.github/workflows/e2e-tests.yml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2453,6 +2453,30 @@ jobs:
24532453
exit 1
24542454
fi
24552455
2456+
# a PHP process spawned during the run - by an extension compiling
2457+
# its own DI container, say - reads the composed ini through PHPRC,
2458+
# so a forked worker that ends must not delete it; the rule spawns
2459+
# one in the main process after every worker has ended
2460+
- operating-system: "ubuntu-latest"
2461+
coverage: "pcov"
2462+
extensions: ""
2463+
script: |
2464+
php ../../bin/phpstan clear-result-cache -c spawn.neon -a SpawnedProcessRule.php
2465+
set +e
2466+
php ../../bin/phpstan analyse -c spawn.neon -a SpawnedProcessRule.php --no-progress --error-format=raw -vvv > out.txt 2>&1
2467+
CODE=$?
2468+
set -e
2469+
cat out.txt
2470+
if ! grep -qE 'Mechanism: +fork' out.txt; then
2471+
echo "FAIL: the workers were not forked"
2472+
exit 1
2473+
fi
2474+
if [ "$CODE" != "0" ]; then
2475+
echo "FAIL: the analysis exited with $CODE"
2476+
exit 1
2477+
fi
2478+
php check.php restarted-for=pcov,opcache pcov-armed=no processes=2+ starts=2
2479+
24562480
# --debug restarts like any other run - it only makes the analysis
24572481
# serial, --xdebug is what keeps xdebug
24582482
- operating-system: "ubuntu-latest"
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace RestartManagerE2E;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Collectors\Collector;
8+
use PHPStan\Node\CollectedDataNode;
9+
use PHPStan\Node\FileNode;
10+
use PHPStan\Rules\Rule;
11+
use PHPStan\Rules\RuleErrorBuilder;
12+
13+
/**
14+
* Spawns a PHP process the way an extension compiling its own DI container
15+
* does. It runs in the main process after every worker has ended, so a worker
16+
* that took the composed ini with it leaves the spawned process with neither
17+
* the ini nor the scan dir - and without the extensions loaded from them.
18+
*
19+
* @implements Rule<CollectedDataNode>
20+
*/
21+
final class SpawnedProcessRule implements Rule
22+
{
23+
24+
public function getNodeType(): string
25+
{
26+
return CollectedDataNode::class;
27+
}
28+
29+
public function processNode(Node $node, Scope $scope): array
30+
{
31+
$output = shell_exec(escapeshellarg(PHP_BINARY) . ' -r ' . escapeshellarg('echo json_encode(get_loaded_extensions());'));
32+
$spawnedExtensions = json_decode((string) $output, true);
33+
if (!is_array($spawnedExtensions)) {
34+
return [RuleErrorBuilder::message(sprintf('The spawned PHP process failed: %s', $output))->identifier('restartManager.spawn')->build()];
35+
}
36+
37+
$missing = array_diff(get_loaded_extensions(), $spawnedExtensions);
38+
if ($missing === []) {
39+
return [];
40+
}
41+
42+
return [
43+
RuleErrorBuilder::message(sprintf(
44+
'The spawned PHP process is missing extensions %s (PHPRC=%s, %s)',
45+
implode(', ', $missing),
46+
(string) getenv('PHPRC'),
47+
is_file((string) getenv('PHPRC')) ? 'exists' : 'does not exist',
48+
))->identifier('restartManager.spawn')->build(),
49+
];
50+
}
51+
52+
}
53+
54+
/**
55+
* CollectedDataNode rules only run when something was collected.
56+
*
57+
* @implements Collector<FileNode, true>
58+
*/
59+
final class FileCollector implements Collector
60+
{
61+
62+
public function getNodeType(): string
63+
{
64+
return FileNode::class;
65+
}
66+
67+
public function processNode(Node $node, Scope $scope): bool
68+
{
69+
return true;
70+
}
71+
72+
}

‎e2e/restart-manager/spawn.neon‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
includes:
2+
- phpstan.neon
3+
4+
rules:
5+
- RestartManagerE2E\SpawnedProcessRule
6+
7+
services:
8+
-
9+
class: RestartManagerE2E\FileCollector
10+
tags:
11+
- phpstan.collector

‎tests/PHPStan/Process/Restart/RestartManagerTest.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,19 @@
66
use PHPUnit\Framework\Attributes\CoversNothing;
77
use PHPUnit\Framework\Attributes\DataProvider;
88
use PHPUnit\Framework\TestCase;
9+
use function fclose;
10+
use function function_exists;
911
use function getenv;
12+
use function is_file;
13+
use function proc_close;
14+
use function proc_open;
1015
use function putenv;
1116
use function sprintf;
17+
use function stream_get_contents;
18+
use function sys_get_temp_dir;
19+
use function tempnam;
20+
use function unlink;
21+
use const PHP_BINARY;
1222

1323
#[CoversNothing]
1424
class RestartManagerTest extends TestCase
@@ -153,4 +163,29 @@ public function testNonRestartingCommandsCollectNoReasons(): void
153163
$this->assertSame([], RestartManager::collectReasons(['bin/phpstan', 'diagnose']));
154164
}
155165

166+
public function testAForkedWorkerDoesNotDeleteTheComposedIni(): void
167+
{
168+
if (!function_exists('pcntl_fork')) {
169+
$this->markTestSkipped('Requires pcntl.');
170+
}
171+
172+
$temporaryIni = tempnam(sys_get_temp_dir(), 'phpstan-restart-ini-test-');
173+
$this->assertNotFalse($temporaryIni);
174+
175+
$environment = getenv();
176+
$environment['PHPSTAN_RESTART_INI'] = $temporaryIni;
177+
$process = proc_open([PHP_BINARY, __DIR__ . '/data/forked-child-ends.php'], [1 => ['pipe', 'w']], $pipes, null, $environment);
178+
$this->assertIsResource($process);
179+
$output = stream_get_contents($pipes[1]);
180+
fclose($pipes[1]);
181+
proc_close($process);
182+
183+
$deletedByOwner = !is_file($temporaryIni);
184+
@unlink($temporaryIni);
185+
186+
// spawned PHP processes read their configuration from it until the run ends
187+
$this->assertSame('kept', $output);
188+
$this->assertTrue($deletedByOwner);
189+
}
190+
156191
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php declare(strict_types = 1);
2+
3+
// The restarted process: it adopts the composed ini named in
4+
// PHPSTAN_RESTART_INI, forks a worker that ends normally, and reports whether
5+
// the ini survived the worker.
6+
7+
use PHPStan\Process\Restart\RestartManager;
8+
9+
require __DIR__ . '/../../../../../vendor/autoload.php';
10+
11+
$temporaryIni = (string) getenv('PHPSTAN_RESTART_INI');
12+
13+
// diagnose never restarts, so this only takes ownership of the ini
14+
RestartManager::run(['bin/phpstan', 'diagnose']);
15+
16+
$pid = pcntl_fork();
17+
if ($pid === 0) {
18+
exit(0);
19+
}
20+
21+
pcntl_waitpid($pid, $status);
22+
23+
echo is_file($temporaryIni) ? 'kept' : 'deleted';

0 commit comments

Comments
 (0)