Skip to content

[Caching] Add failing reproducer for partially written cache reads - #8511

Open
calebdw wants to merge 1 commit into
rectorphp:partial-cachefrom
calebdw:atomic-cache-write-test
Open

calebdw wants to merge 1 commit into
rectorphp:partial-cachefrom
calebdw:atomic-cache-write-test

Conversation

@calebdw

@calebdw calebdw commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Targets partial-cache so it lands inside #8426 rather than alongside it. Test only — no production change.

Addresses the one thing blocking that PR:

@TomasVotruba: We'll need a failing reproducer in our CI, so we avoid changing this back and forth.

Fails on main's copy(), passes on this branch's rename(). No threads, no timing, no sleep.

The test opens a read handle on a cache file, saves over the same key, and then reads from the handle it opened before the save — the way a parallel worker holds the configuration_hash file open while require-ing it during container boot. copy() truncates the destination and streams into the very inode that reader is holding, so the reader's file changes underneath it. rename() publishes a new inode and leaves the reader on the complete file it opened.

On main:

1) Rector\Tests\Caching\ValueObject\Storage\FileCacheStorageTest::testSaveLeavesAConcurrentReaderOnACompleteFile
Failed asserting that two strings are identical.
--- Expected
+++ Actual
 return \Rector\Caching\ValueObject\CacheItem::__set_state(array(
    'variableKey' => 'TEST',
-   'data' => 'first',
+   'data' => 'second',
 ));

What it does not prove

It is not a reproduction of the crash. It pins the precondition — a writer mutating a file under a live reader — not a torn read. With a payload this small copy() finishes before the read, so you get complete-but-wrong content rather than the syntax error, unexpected string content / Child process error failure seen in the wild. Making an actual torn read deterministic needs the interleaving, which I don't think is achievable in a test.

What it does give you is a guard that fails the moment copy() comes back, which I read as the point of the request.

Checks

  • vendor/bin/phpunit tests/Caching — 14/14 green on this branch, 1 failure on main
  • composer check-cs — clean

Context and the Linux reproduction that led here: #8426 (comment)

Every worker in a parallel run boots its own container, and each one
require()s the configuration_hash cache file while another worker may be
saving over it. copy() truncates the destination and streams into it, so a
reader can catch a prefix of the new file and die on it:

    {"fatal_errors":["syntax error, unexpected string content ..."]}
    [ERROR] Could not process some files, due to: "Child process error".

The interleaving that produces a torn read is not reproducible on demand,
but the precondition for it is, with no threads and no timing: open a
reader on the cache file, save over it, and read. copy() writes through
the inode the reader is holding, so the reader's file changes underneath
it; an atomic replace leaves that reader on the whole file it opened.

Fails on copy(), passes on rename().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant