Skip to content

Commit de278eb

Browse files
SanderMullerclaude
andcommitted
Discard a damaged framed cache instead of reading it half way
A cache file that is this format but truncated - which is what a process killed during the save leaves behind, since the file is streamed to its final path - was read as far as it went and the missing frames returned as null. `is_array()` on the assembled array then passed, and the first missing section surfaced as a TypeError from isMetaDifferent() far away from the cause. Every format violation now throws, so restore() discards the file and analyses everything, which is what the var_export format did by way of a ParseError from the include. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent c27d505 commit de278eb

7 files changed

Lines changed: 83 additions & 13 deletions

File tree

.github/workflows/e2e-tests.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,11 @@ jobs:
124124
mv src/Foo.php.orig src/Foo.php
125125
echo -n > phpstan-baseline.neon
126126
../../bin/phpstan -vvv
127+
- script: |
128+
cd e2e/result-cache-truncated
129+
../../bin/phpstan -vvv
130+
php truncate.php
131+
../../bin/phpstan -vvv
127132
- script: |
128133
cd e2e/bug-14514
129134
composer install

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@
1515
tmp/.memory_limit
1616
e2e/bashunit
1717
/.phpbench
18+
/e2e/result-cache-truncated/tmp
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
parameters:
2+
level: 8
3+
tmpDir: tmp
4+
paths:
5+
- src
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace TestResultCacheTruncated;
4+
5+
class Bar
6+
{
7+
8+
public function doBar(): string
9+
{
10+
return 'bar';
11+
}
12+
13+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace TestResultCacheTruncated;
4+
5+
class Foo
6+
{
7+
8+
public function doFoo(Bar $bar): string
9+
{
10+
return $bar->doBar();
11+
}
12+
13+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php declare(strict_types = 1);
2+
3+
// A process killed while the result cache is being written leaves a partial file at the final
4+
// path, because the cache is streamed there rather than written atomically. Cutting the file
5+
// inside the first section's payload is the shape that used to be read as a half-populated
6+
// cache instead of a damaged one.
7+
$file = __DIR__ . '/tmp/resultCache.php';
8+
$contents = file_get_contents($file);
9+
if ($contents === false) {
10+
throw new RuntimeException('No result cache at ' . $file);
11+
}
12+
13+
file_put_contents($file, substr($contents, 0, 200));

src/Analyser/ResultCache/ResultCacheManager.php

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use PHPStan\ShouldNotHappenException;
2929
use ReflectionClass;
3030
use ReflectionException;
31+
use RuntimeException;
3132
use Throwable;
3233
use function array_diff;
3334
use function array_fill_keys;
@@ -1333,7 +1334,7 @@ private function readCacheFile(string $cacheFilePath): ?array
13331334

13341335
$parts = explode(' ', $header, 2);
13351336
if (count($parts) !== 2) {
1336-
return null;
1337+
throw new RuntimeException(sprintf('Malformed frame header "%s".', $header));
13371338
}
13381339

13391340
[$name, $size] = $parts;
@@ -1343,12 +1344,7 @@ private function readCacheFile(string $cacheFilePath): ?array
13431344
continue;
13441345
}
13451346

1346-
$entries = $this->readEntryFrames($handle, (int) $size);
1347-
if ($entries === null) {
1348-
return null;
1349-
}
1350-
1351-
$data[substr($name, 0, -1)] = $entries;
1347+
$data[substr($name, 0, -1)] = $this->readEntryFrames($handle, (int) $size);
13521348
}
13531349

13541350
return $data;
@@ -1359,20 +1355,20 @@ private function readCacheFile(string $cacheFilePath): ?array
13591355

13601356
/**
13611357
* @param resource $handle
1362-
* @return array<mixed>|null
1358+
* @return array<mixed>
13631359
*/
1364-
private function readEntryFrames($handle, int $count): ?array
1360+
private function readEntryFrames($handle, int $count): array
13651361
{
13661362
$entries = [];
13671363
for ($i = 0; $i < $count; $i++) {
13681364
$length = fgets($handle);
13691365
if ($length === false) {
1370-
return null;
1366+
throw new RuntimeException(sprintf('Cache file ended after %d of %d entries.', $i, $count));
13711367
}
13721368

13731369
$entry = $this->readFrame($handle, (int) rtrim($length, "\n"));
13741370
if (!is_array($entry)) {
1375-
return null;
1371+
throw new RuntimeException('An entry frame did not contain an array.');
13761372
}
13771373

13781374
foreach ($entry as $key => $value) {
@@ -1384,17 +1380,41 @@ private function readEntryFrames($handle, int $count): ?array
13841380
}
13851381

13861382
/**
1383+
* A frame's payload, or an exception when the file does not hold one.
1384+
*
1385+
* Every failure here means a cache file that is this format but damaged - a process killed
1386+
* mid-save leaves exactly that, since the file is written in place. restore() turns the
1387+
* exception into a discarded cache and a full analysis, the same way it handles the parse
1388+
* error an incomplete var_export'd file used to produce. Returning a value instead would
1389+
* hand a half-read cache to the caller, where the missing pieces surface as type errors far
1390+
* from the cause.
1391+
*
1392+
* false is treated as failure because unserialize() reports failure that way and no value in
1393+
* the cache is a bare false: the sections are arrays and lastFullAnalysisTime is an int.
1394+
*
13871395
* @param resource $handle
13881396
*/
13891397
private function readFrame($handle, int $length): mixed
13901398
{
13911399
if ($length <= 0) {
1392-
return null;
1400+
throw new RuntimeException(sprintf('Frame length %d is not positive.', $length));
13931401
}
13941402

13951403
$blob = fread($handle, $length);
1404+
if ($blob === false || strlen($blob) !== $length) {
1405+
throw new RuntimeException(sprintf(
1406+
'Expected a %d byte frame, read %d bytes.',
1407+
$length,
1408+
$blob === false ? 0 : strlen($blob),
1409+
));
1410+
}
1411+
1412+
$value = @unserialize($blob);
1413+
if ($value === false) {
1414+
throw new RuntimeException(sprintf('A %d byte frame could not be unserialized.', $length));
1415+
}
13961416

1397-
return $blob === false ? null : @unserialize($blob);
1417+
return $value;
13981418
}
13991419

14001420
/**

0 commit comments

Comments
 (0)