Skip to content

Commit e3d5d8b

Browse files
committed
refactor: use shared utilities in parallel processing classes
- Replace blocking pcntl_waitpid with ProcessManager timeout (300s) - Replace tempnam with ProcessManager secure temp files (0600) - Replace duplicated detectCpuCount with CpuDetector Affected classes: - ForkingRenderer - ParallelCompiler - ParallelParseDirectoryHandler - SingleForkPipeline This fixes HIGH severity issues: stuck builds now timeout after 5min, and temp files are no longer world-readable.
1 parent d497cbe commit e3d5d8b

4 files changed

Lines changed: 374 additions & 159 deletions

File tree

packages/typo3-guides-extension/src/Compiler/ParallelCompiler.php

Lines changed: 73 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
use phpDocumentor\Guides\Nodes\DocumentTree\ExternalEntryNode;
1616
use Psr\Log\LoggerInterface;
1717
use SplPriorityQueue;
18+
use T3Docs\GuidesExtension\Compiler\Cache\IncrementalBuildCache;
19+
use T3Docs\GuidesExtension\Util\CpuDetector;
20+
use T3Docs\GuidesExtension\Util\ProcessManager;
1821

1922
use function array_chunk;
2023
use function count;
@@ -23,14 +26,8 @@
2326
use function function_exists;
2427
use function iterator_to_array;
2528
use function pcntl_fork;
26-
use function pcntl_waitpid;
27-
use function pcntl_wexitstatus;
28-
use function pcntl_wifexited;
2929
use function serialize;
3030
use function sprintf;
31-
use function sys_get_temp_dir;
32-
use function tempnam;
33-
use function unlink;
3431
use function unserialize;
3532

3633
/**
@@ -85,6 +82,7 @@ public function __construct(
8582
private readonly Compiler $sequentialCompiler,
8683
iterable $passes,
8784
NodeTransformerFactory $nodeTransformerFactory,
85+
private readonly ?IncrementalBuildCache $incrementalCache = null,
8886
private readonly ?LoggerInterface $logger = null,
8987
?int $workerCount = null,
9088
) {
@@ -218,7 +216,7 @@ private function runCollectionPhase(array $documents, CompilerContext $compilerC
218216
continue;
219217
}
220218

221-
$tempFile = tempnam(sys_get_temp_dir(), 'compile_collect_' . $workerId . '_');
219+
$tempFile = ProcessManager::createSecureTempFile('compile_collect_' . $workerId . '_');
222220
if ($tempFile === false) {
223221
$this->logger?->error('Failed to create temp file, falling back to sequential');
224222
return [$this->runSequentially($documents, $compilerContext), []];
@@ -230,7 +228,7 @@ private function runCollectionPhase(array $documents, CompilerContext $compilerC
230228
if ($pid === -1) {
231229
$this->logger?->error('pcntl_fork failed, falling back to sequential');
232230
foreach ($tempFiles as $tf) {
233-
@unlink($tf);
231+
ProcessManager::cleanupTempFile($tf);
234232
}
235233
return [$this->runSequentially($documents, $compilerContext), []];
236234
}
@@ -244,18 +242,14 @@ private function runCollectionPhase(array $documents, CompilerContext $compilerC
244242
$childPids[$workerId] = $pid;
245243
}
246244

247-
// Wait for children and collect results
245+
// Wait for children with timeout and collect results
246+
$waitResult = ProcessManager::waitForChildrenWithTimeout($childPids);
248247
$allDocuments = [];
249248
$allResults = [];
250249

251250
foreach ($childPids as $workerId => $pid) {
252-
$status = 0;
253-
pcntl_waitpid($pid, $status);
254-
255-
// pcntl_waitpid always sets $status to an int
256-
assert(is_int($status));
257-
258-
if (pcntl_wifexited($status) && pcntl_wexitstatus($status) === 0) {
251+
// Only read results from successful workers
252+
if (in_array($workerId, $waitResult['successes'], true)) {
259253
$serialized = file_get_contents($tempFiles[$workerId]);
260254
if ($serialized !== false && $serialized !== '') {
261255
$data = unserialize($serialized);
@@ -274,7 +268,14 @@ private function runCollectionPhase(array $documents, CompilerContext $compilerC
274268
}
275269
}
276270

277-
@unlink($tempFiles[$workerId]);
271+
ProcessManager::cleanupTempFile($tempFiles[$workerId]);
272+
}
273+
274+
// Log any failures
275+
if ($waitResult['failures'] !== []) {
276+
foreach ($waitResult['failures'] as $workerId => $reason) {
277+
$this->logger?->warning(sprintf('Collection worker %d failed: %s', $workerId, $reason));
278+
}
278279
}
279280

280281
// Preserve document order
@@ -468,7 +469,7 @@ private function runResolutionPhase(array $documents, CompilerContext $compilerC
468469
continue;
469470
}
470471

471-
$tempFile = tempnam(sys_get_temp_dir(), 'compile_resolve_' . $workerId . '_');
472+
$tempFile = ProcessManager::createSecureTempFile('compile_resolve_' . $workerId . '_');
472473
if ($tempFile === false) {
473474
return $this->runResolutionSequentially($documents, $compilerContext);
474475
}
@@ -478,7 +479,7 @@ private function runResolutionPhase(array $documents, CompilerContext $compilerC
478479

479480
if ($pid === -1) {
480481
foreach ($tempFiles as $tf) {
481-
@unlink($tf);
482+
ProcessManager::cleanupTempFile($tf);
482483
}
483484
return $this->runResolutionSequentially($documents, $compilerContext);
484485
}
@@ -491,30 +492,61 @@ private function runResolutionPhase(array $documents, CompilerContext $compilerC
491492
$childPids[$workerId] = $pid;
492493
}
493494

494-
// Collect results
495+
// Wait for children with timeout and collect results
496+
$waitResult = ProcessManager::waitForChildrenWithTimeout($childPids);
495497
$allDocuments = [];
496-
foreach ($childPids as $workerId => $pid) {
497-
$status = 0;
498-
pcntl_waitpid($pid, $status);
498+
$cacheStates = [];
499499

500-
// pcntl_waitpid always sets $status to an int
501-
assert(is_int($status));
502-
503-
if (pcntl_wifexited($status) && pcntl_wexitstatus($status) === 0) {
500+
foreach ($childPids as $workerId => $pid) {
501+
// Only read results from successful workers
502+
if (in_array($workerId, $waitResult['successes'], true)) {
504503
$serialized = file_get_contents($tempFiles[$workerId]);
505504
if ($serialized !== false && $serialized !== '') {
506-
$batchResult = unserialize($serialized);
507-
if (is_array($batchResult)) {
508-
foreach ($batchResult as $doc) {
509-
if ($doc instanceof DocumentNode) {
510-
$allDocuments[$doc->getFilePath()] = $doc;
505+
$data = unserialize($serialized);
506+
if (is_array($data)) {
507+
// New format with cache state
508+
if (isset($data['documents'])) {
509+
foreach ($data['documents'] as $doc) {
510+
if ($doc instanceof DocumentNode) {
511+
$allDocuments[$doc->getFilePath()] = $doc;
512+
}
513+
}
514+
// Collect cache state for merging
515+
if (isset($data['cacheState']) && is_array($data['cacheState'])) {
516+
$cacheStates[] = $data['cacheState'];
517+
}
518+
} else {
519+
// Legacy format (just documents array)
520+
foreach ($data as $doc) {
521+
if ($doc instanceof DocumentNode) {
522+
$allDocuments[$doc->getFilePath()] = $doc;
523+
}
511524
}
512525
}
513526
}
514527
}
515528
}
516529

517-
@unlink($tempFiles[$workerId]);
530+
ProcessManager::cleanupTempFile($tempFiles[$workerId]);
531+
}
532+
533+
// Log any failures
534+
if ($waitResult['failures'] !== []) {
535+
foreach ($waitResult['failures'] as $workerId => $reason) {
536+
$this->logger?->warning(sprintf('Resolution worker %d failed: %s', $workerId, $reason));
537+
}
538+
}
539+
540+
// Merge cache states from all children
541+
if ($this->incrementalCache !== null && $cacheStates !== []) {
542+
foreach ($cacheStates as $state) {
543+
$this->incrementalCache->mergeState($state);
544+
}
545+
$this->logger?->debug(sprintf(
546+
'Merged cache states from %d workers, now have %d exports',
547+
count($cacheStates),
548+
count($this->incrementalCache->getAllExports())
549+
));
518550
}
519551

520552
// Preserve order
@@ -541,7 +573,13 @@ private function processResolutionBatch(
541573
$batch = $pass->run($batch, $compilerContext);
542574
}
543575

544-
file_put_contents($tempFile, serialize($batch));
576+
// Serialize documents and cache state (if cache is available)
577+
$data = [
578+
'documents' => $batch,
579+
'cacheState' => $this->incrementalCache?->extractState() ?? [],
580+
];
581+
582+
file_put_contents($tempFile, serialize($data));
545583
}
546584

547585
/**
@@ -698,25 +736,7 @@ private function shouldFork(int $documentCount): bool
698736

699737
private function detectCpuCount(): int
700738
{
701-
if (is_file('/proc/cpuinfo')) {
702-
$cpuinfo = file_get_contents('/proc/cpuinfo');
703-
if ($cpuinfo !== false) {
704-
$count = substr_count($cpuinfo, 'processor');
705-
if ($count > 0) {
706-
return min($count, 8);
707-
}
708-
}
709-
}
710-
711-
$nproc = @shell_exec('nproc 2>/dev/null');
712-
if ($nproc !== null && $nproc !== false) {
713-
$count = (int) trim($nproc);
714-
if ($count > 0) {
715-
return min($count, 8);
716-
}
717-
}
718-
719-
return 4;
739+
return CpuDetector::detectCores();
720740
}
721741

722742
public function setParallelEnabled(bool $enabled): void

packages/typo3-guides-extension/src/Parser/ParallelParseDirectoryHandler.php

Lines changed: 16 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
use phpDocumentor\Guides\Settings\SettingsManager;
1818
use Psr\EventDispatcher\EventDispatcherInterface;
1919
use Psr\Log\LoggerInterface;
20+
use T3Docs\GuidesExtension\Util\CpuDetector;
21+
use T3Docs\GuidesExtension\Util\ProcessManager;
2022

2123
use function array_chunk;
2224
use function array_map;
@@ -28,13 +30,7 @@
2830
use function file_put_contents;
2931
use function function_exists;
3032
use function pcntl_fork;
31-
use function pcntl_waitpid;
32-
use function pcntl_wifexited;
33-
use function pcntl_wexitstatus;
3433
use function serialize;
35-
use function sys_get_temp_dir;
36-
use function tempnam;
37-
use function unlink;
3834
use function unserialize;
3935

4036
/**
@@ -165,8 +161,8 @@ private function parseInParallel(ParseDirectoryCommand $command, array $files, s
165161
continue;
166162
}
167163

168-
// Create temp file for this worker's results
169-
$tempFile = tempnam(sys_get_temp_dir(), 'parse_' . $workerId . '_');
164+
// Create secure temp file for this worker's results
165+
$tempFile = ProcessManager::createSecureTempFile('parse_' . $workerId . '_');
170166
if ($tempFile === false) {
171167
$this->logger?->error('Failed to create temp file, falling back to sequential');
172168
return $this->parseSequentially($command, $files, $indexName);
@@ -179,7 +175,7 @@ private function parseInParallel(ParseDirectoryCommand $command, array $files, s
179175
// Fork failed - clean up and fall back
180176
$this->logger?->error('pcntl_fork failed, falling back to sequential parsing');
181177
foreach ($tempFiles as $tf) {
182-
@unlink($tf);
178+
ProcessManager::cleanupTempFile($tf);
183179
}
184180
return $this->parseSequentially($command, $files, $indexName);
185181
}
@@ -194,20 +190,14 @@ private function parseInParallel(ParseDirectoryCommand $command, array $files, s
194190
$childPids[$workerId] = $pid;
195191
}
196192

197-
// Parent: wait for all children and collect results
193+
// Parent: wait for all children with timeout and collect results
194+
$waitResult = ProcessManager::waitForChildrenWithTimeout($childPids);
198195
$documents = [];
199-
$failures = [];
200196

201197
foreach ($childPids as $workerId => $pid) {
202-
$status = 0;
203-
pcntl_waitpid($pid, $status);
204-
205-
// pcntl_waitpid always sets $status to an int
206-
assert(is_int($status));
207-
if (pcntl_wifexited($status) && pcntl_wexitstatus($status) === 0) {
208-
// Read results from temp file
209-
$tempFile = $tempFiles[$workerId];
210-
$serialized = file_get_contents($tempFile);
198+
// Only read results from successful workers
199+
if (in_array($workerId, $waitResult['successes'], true)) {
200+
$serialized = file_get_contents($tempFiles[$workerId]);
211201
if ($serialized !== false && $serialized !== '') {
212202
$batchDocs = unserialize($serialized);
213203
if (is_array($batchDocs)) {
@@ -218,19 +208,16 @@ private function parseInParallel(ParseDirectoryCommand $command, array $files, s
218208
}
219209
}
220210
}
221-
} else {
222-
$failures[] = $workerId;
223211
}
224212

225213
// Clean up temp file
226-
@unlink($tempFiles[$workerId]);
214+
ProcessManager::cleanupTempFile($tempFiles[$workerId]);
227215
}
228216

229-
if ($failures !== []) {
230-
$this->logger?->warning(sprintf(
231-
'Parallel parsing: %d workers failed',
232-
count($failures)
233-
));
217+
if ($waitResult['failures'] !== []) {
218+
foreach ($waitResult['failures'] as $workerId => $reason) {
219+
$this->logger?->warning(sprintf('Parse worker %d failed: %s', $workerId, $reason));
220+
}
234221
}
235222

236223
$this->logger?->info(sprintf(
@@ -333,24 +320,6 @@ private function getDirectoryIndexFile(ParseDirectoryCommand $command): string
333320

334321
private function detectCpuCount(): int
335322
{
336-
if (is_file('/proc/cpuinfo')) {
337-
$cpuinfo = file_get_contents('/proc/cpuinfo');
338-
if ($cpuinfo !== false) {
339-
$count = substr_count($cpuinfo, 'processor');
340-
if ($count > 0) {
341-
return min($count, 8);
342-
}
343-
}
344-
}
345-
346-
$nproc = @shell_exec('nproc 2>/dev/null');
347-
if ($nproc !== null && $nproc !== false) {
348-
$count = (int) trim($nproc);
349-
if ($count > 0) {
350-
return min($count, 8);
351-
}
352-
}
353-
354-
return 4;
323+
return CpuDetector::detectCores();
355324
}
356325
}

0 commit comments

Comments
 (0)