diff --git a/src/Command/FixerApplication.php b/src/Command/FixerApplication.php index 2ebe31762bd..a98b7fa1d12 100644 --- a/src/Command/FixerApplication.php +++ b/src/Command/FixerApplication.php @@ -11,7 +11,6 @@ use GuzzleHttp\RequestOptions; use Nette\Utils\Json; use Phar; -use PHPStan\Analyser\Ignore\IgnoredErrorHelper; use PHPStan\Analyser\InternalError; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; @@ -19,18 +18,14 @@ use PHPStan\File\FileMonitorResult; use PHPStan\File\FileReader; use PHPStan\File\FileWriter; -use PHPStan\File\PathNotFoundException; use PHPStan\Internal\ComposerHelper; use PHPStan\Internal\DirectoryCreator; use PHPStan\Internal\DirectoryCreatorException; use PHPStan\Internal\HttpClientFactory; -use PHPStan\Parallel\ForkParallelChecker; use PHPStan\PhpDoc\StubFilesProvider; -use PHPStan\Process\ForkedProcessPromise; use PHPStan\Process\ProcessCanceledException; use PHPStan\Process\ProcessCrashedException; use PHPStan\Process\ProcessHelper; -use PHPStan\Process\ProcessPromise; use PHPStan\Process\SpawnedProcessPromise; use PHPStan\ShouldNotHappenException; use React\ChildProcess\Process; @@ -76,7 +71,6 @@ final class FixerApplication */ public function __construct( private FileMonitor $fileMonitor, - private IgnoredErrorHelper $ignoredErrorHelper, private StubFilesProvider $stubFilesProvider, #[AutowiredParameter] private array $analysedPaths, @@ -97,8 +91,6 @@ public function __construct( #[AutowiredParameter] private string $usedLevel, private HttpClientFactory $httpClientFactory, - private ForkParallelChecker $forkParallelChecker, - private FixerWorkerRunner $fixerWorkerRunner, ) { } @@ -121,7 +113,7 @@ public function run( /** @var int<0, 65535> $serverPort */ $serverPort = parse_url($serverAddress, PHP_URL_PORT); - $server->on('connection', function (ConnectionInterface $connection) use ($loop, $inceptionResult, $projectConfigFile, $input, $output, $mainScript, $filesCount): void { + $server->on('connection', function (ConnectionInterface $connection) use ($loop, $projectConfigFile, $input, $output, $mainScript, $filesCount): void { // phpcs:disable SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly $jsonInvalidUtf8Ignore = defined('JSON_INVALID_UTF8_IGNORE') ? JSON_INVALID_UTF8_IGNORE : 0; // phpcs:enable @@ -164,14 +156,13 @@ public function run( $this->analyse( $loop, - $inceptionResult, $mainScript, $projectConfigFile, $input, $encoder, ); - $this->monitorFileChanges($loop, function (FileMonitorResult $changes) use ($loop, $inceptionResult, $mainScript, $projectConfigFile, $input, $encoder): void { + $this->monitorFileChanges($loop, function (FileMonitorResult $changes) use ($loop, $mainScript, $projectConfigFile, $input, $encoder): void { if ($this->processInProgress !== null) { $this->processInProgress->cancel(); $this->processInProgress = null; @@ -185,7 +176,6 @@ public function run( $this->analyse( $loop, - $inceptionResult, $mainScript, $projectConfigFile, $input, @@ -424,18 +414,12 @@ private function monitorFileChanges(LoopInterface $loop, callable $hasChangesCal private function analyse( LoopInterface $loop, - InceptionResult $inceptionResult, string $mainScript, ?string $projectConfigFile, InputInterface $input, Encoder $phpstanFixerEncoder, ): void { - $ignoredErrorHelperResult = $this->ignoredErrorHelper->initialize(); - if (count($ignoredErrorHelperResult->getErrors()) > 0) { - throw new ShouldNotHappenException(); - } - // TCP server for fixer:worker (TCP client) $server = new TcpServer('127.0.0.1:0', $loop); /** @var string $serverAddress */ @@ -453,16 +437,20 @@ private function analyse( }); }); - $process = $this->createProcessPromise( - $this->forkParallelChecker->isSupported(), - $loop, - $server, + // Always a fresh process, never a fork of this one: this process runs as long as + // PHPStan Pro does, so its container and config are the ones it started with. + // A spawned fixer:worker reads the config files again, which picks up edits + // like errors added to the baseline. It still forks its own analysis workers. + $process = new SpawnedProcessPromise($loop, ProcessHelper::getWorkerCommand( $mainScript, + 'fixer:worker', $projectConfigFile, + [ + '--server-port', + (string) $serverPort, + ], $input, - $serverPort, - $inceptionResult, - ); + )); $this->processInProgress = $process->run(); $this->processInProgress->then(function () use ($server): void { @@ -573,50 +561,4 @@ private function getStubFiles(): array return $stubFiles; } - /** - * @param int<0, 65535> $serverPort - */ - private function createProcessPromise( - bool $useFork, - LoopInterface $loop, - TcpServer $server, - string $mainScript, - ?string $projectConfigFile, - InputInterface $input, - int $serverPort, - InceptionResult $inceptionResult, - ): ProcessPromise - { - if ($useFork) { - try { - [$inceptionFiles, $isOnlyFiles] = $inceptionResult->getFiles(); - } catch (PathNotFoundException | InceptionNotSuccessfulException) { - throw new ShouldNotHappenException(); - } - - return new ForkedProcessPromise( - $loop, - $this->fixerWorkerRunner, - $server, - $inceptionFiles, - $isOnlyFiles, - $inceptionResult->getProjectConfigArray(), - $projectConfigFile, - $serverPort, - $input, - ); - } - - return new SpawnedProcessPromise($loop, ProcessHelper::getWorkerCommand( - $mainScript, - 'fixer:worker', - $projectConfigFile, - [ - '--server-port', - (string) $serverPort, - ], - $input, - )); - } - } diff --git a/src/Command/FixerWorkerCommand.php b/src/Command/FixerWorkerCommand.php index 60357599096..0461df1dc31 100644 --- a/src/Command/FixerWorkerCommand.php +++ b/src/Command/FixerWorkerCommand.php @@ -98,9 +98,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int throw new ShouldNotHappenException(); } - // Everything after the boot lives in FixerWorkerRunner so a - // pcntl_fork()-ed child can reuse it without re-booting (see - // FixerApplication). $fixerWorkerRunner = $container->getByType(FixerWorkerRunner::class); return $fixerWorkerRunner->run( diff --git a/src/Command/FixerWorkerRunner.php b/src/Command/FixerWorkerRunner.php index 182fb125d9d..f0205dda5da 100644 --- a/src/Command/FixerWorkerRunner.php +++ b/src/Command/FixerWorkerRunner.php @@ -39,11 +39,9 @@ /** * The PHPStan Pro worker logic that runs *after* the application boot. * - * Extracted from FixerWorkerCommand so it can be reused without re-booting: a - * pcntl_fork()-ed child (see ForkedProcessPromise) inherits the already-booted - * DI container and calls run() directly, while FixerWorkerCommand still calls - * it after the expensive CommandHelper::begin() boot of a freshly spawned - * process. + * FixerWorkerCommand calls it after the CommandHelper::begin() boot of the + * process FixerApplication spawns for each analysis. The analysis itself + * forks its workers where that is supported (see ParallelAnalyser). * * It connects back to FixerApplication's TCP server, restores the result * cache, analyses the changed files and streams the results. @@ -82,8 +80,6 @@ public function run( throw new ShouldNotHappenException(); } - // Always a fresh event loop: in a forked child the parent's inherited - // loop must never be touched. $loop = new StreamSelectLoop(); $tcpConnector = new TcpConnector($loop); $tcpConnector->connect(sprintf('127.0.0.1:%d', $serverPort))->then(function (ConnectionInterface $connection) use ($errorOutput, $inceptionFiles, $isOnlyFiles, $projectConfigArray, $configuration, $input, $ignoredErrorHelperResult, $loop): void { diff --git a/src/Process/ForkedProcessPromise.php b/src/Process/ForkedProcessPromise.php deleted file mode 100644 index 2f4158d77cc..00000000000 --- a/src/Process/ForkedProcessPromise.php +++ /dev/null @@ -1,231 +0,0 @@ - */ - private Deferred $deferred; - - private ?int $childPid = null; - - /** @var resource|null */ - private $stdOut = null; - - /** @var resource|null */ - private $stdErr = null; - - private ?TimerInterface $waitTimer = null; - - private bool $canceled = false; - - /** - * @param string[] $inceptionFiles - * @param mixed[]|null $projectConfigArray - */ - public function __construct( - private LoopInterface $loop, - private FixerWorkerRunner $fixerWorkerRunner, - private TcpServer $server, - private array $inceptionFiles, - private bool $isOnlyFiles, - private ?array $projectConfigArray, - private ?string $configuration, - private int $serverPort, - private InputInterface $input, - ) - { - $this->deferred = new Deferred(function (): void { - $this->cancel(); - }); - } - - /** - * @return PromiseInterface - */ - public function run(): PromiseInterface - { - // Created before the fork so the parent can read what the child wrote. - $tmpStdOut = tmpfile(); - if ($tmpStdOut === false) { - throw new ShouldNotHappenException('Failed creating temp file for stdout.'); - } - $tmpStdErr = tmpfile(); - if ($tmpStdErr === false) { - fclose($tmpStdOut); - throw new ShouldNotHappenException('Failed creating temp file for stderr.'); - } - $this->stdOut = $tmpStdOut; - $this->stdErr = $tmpStdErr; - - $pid = pcntl_fork(); - - if ($pid === -1) { - $this->closeCaptureFiles(); - // Deferred so it runs after FixerApplication has stored the promise. - $this->loop->futureTick(function (): void { - $this->deferred->reject(new ProcessCrashedException('pcntl_fork() failed.')); - }); - - return $this->deferred->promise(); - } - - if ($pid === 0) { - // Child: drop the inherited listening socket immediately, then run - // the worker on its own fresh event loop and never return. - $this->server->close(); - ForkedChildCrashReporter::install($tmpStdErr); - // after the crash reporter: its shutdown function must run first - ForkedChildTerminator::install(); - // The worker writes its errors into the capture the parent reads, - // like a spawned worker whose stderr is the capture itself. - $childErrorStream = new StreamOutput($tmpStdErr); - $exitCode = $this->fixerWorkerRunner->run( - new SymfonyOutput($childErrorStream, new SymfonyStyle(new ErrorsConsoleStyle(new StringInput(''), $childErrorStream))), - $this->inceptionFiles, - $this->isOnlyFiles, - $this->projectConfigArray, - $this->configuration, - $this->serverPort, - $this->input, - ); - exit($exitCode); - } - - // Parent: poll for the child to exit and resolve/reject accordingly. - $this->childPid = $pid; - $this->waitTimer = $this->loop->addPeriodicTimer(self::WAITPID_POLL_INTERVAL, function () use ($pid): void { - $status = 0; - $result = pcntl_waitpid($pid, $status, WNOHANG); - if ($result === 0) { - return; - } - - $this->cancelWaitTimer(); - - $stdOut = ''; - if ($this->stdOut !== null) { - rewind($this->stdOut); - $stdOut = (string) stream_get_contents($this->stdOut); - } - $stdErr = ''; - if ($this->stdErr !== null) { - rewind($this->stdErr); - $stdErr = (string) stream_get_contents($this->stdErr); - } - $this->closeCaptureFiles(); - - if ($this->canceled) { - // cancel() already rejected the promise; just reap the child. - return; - } - - $exitCode = null; - if ($result > 0 && pcntl_wifexited($status)) { - $exitStatus = pcntl_wexitstatus($status); - if ($exitStatus !== false) { - $exitCode = $exitStatus; - } - } - - if ($exitCode === 0) { - $this->deferred->resolve($stdOut); - return; - } - - // A child killed by a signal writes nothing of its own, so naming - // the signal is all the crash report there is. - if ($result > 0 && pcntl_wifsignaled($status)) { - $signal = pcntl_wtermsig($status); - if ($signal !== false) { - $this->deferred->reject(new ProcessCrashedException(sprintf( - "Child process was killed by signal %s.\n%s", - TerminationSignal::describe($signal), - $stdOut . $stdErr, - ))); - return; - } - } - - $this->deferred->reject(new ProcessCrashedException($stdOut . $stdErr)); - }); - - return $this->deferred->promise(); - } - - private function cancel(): void - { - if ($this->childPid === null) { - throw new ShouldNotHappenException('Cancelling process before running'); - } - $this->canceled = true; - // SIGTERM the child; the waitpid poll timer keeps running so it still - // gets reaped (otherwise: zombie). - posix_kill($this->childPid, SIGTERM); - $this->deferred->reject(new ProcessCanceledException()); - } - - private function cancelWaitTimer(): void - { - if ($this->waitTimer === null) { - return; - } - - $this->loop->cancelTimer($this->waitTimer); - $this->waitTimer = null; - } - - private function closeCaptureFiles(): void - { - if ($this->stdOut !== null) { - fclose($this->stdOut); - $this->stdOut = null; - } - if ($this->stdErr === null) { - return; - } - - fclose($this->stdErr); - $this->stdErr = null; - } - -} diff --git a/src/Process/ProcessPromise.php b/src/Process/ProcessPromise.php deleted file mode 100644 index 1d588c42073..00000000000 --- a/src/Process/ProcessPromise.php +++ /dev/null @@ -1,24 +0,0 @@ - - */ - public function run(): PromiseInterface; - -} diff --git a/src/Process/SpawnedProcessPromise.php b/src/Process/SpawnedProcessPromise.php index 4504e4161d0..5530080913d 100644 --- a/src/Process/SpawnedProcessPromise.php +++ b/src/Process/SpawnedProcessPromise.php @@ -13,12 +13,11 @@ use function tmpfile; /** - * ProcessPromise backed by a freshly spawned PHP process (react/child-process). - * - * @see ForkedProcessPromise for the pcntl_fork()-based alternative that skips - * the per-worker application re-boot. + * A freshly spawned PHP process (react/child-process) as a promise that resolves + * with its output on success and rejects with ProcessCrashedException or + * ProcessCanceledException otherwise. */ -final class SpawnedProcessPromise implements ProcessPromise +final class SpawnedProcessPromise { /** @var Deferred */