From 7c8d927aa8720fb4fd6dbc9c91eb54e771c327d0 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 29 Sep 2026 14:07:39 +0200 Subject: [PATCH 1/4] [Caching] Use FileSystem::writeAtomic() in FileCacheStorage to avoid partial cache reads on parallel run --- composer.json | 2 +- .../ValueObject/Storage/FileCacheStorage.php | 17 +++----------- .../Storage/FileCacheStorageTest.php | 23 +++++++++++++++++++ 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/composer.json b/composer.json index 81344995e6a..b06552a5721 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,7 @@ "doctrine/inflector": "^2.1", "entropy/entropy": "^0.4.12", "fidry/cpu-core-counter": "^1.1", - "nette/utils": "^4.1.4", + "nette/utils": "^4.1.5", "nikic/php-parser": "^5.9", "ondram/ci-detector": "^4.2", "phpstan/phpdoc-parser": "^2.3.3", diff --git a/src/Caching/ValueObject/Storage/FileCacheStorage.php b/src/Caching/ValueObject/Storage/FileCacheStorage.php index b883cb4b9ae..ad0e3abc1db 100644 --- a/src/Caching/ValueObject/Storage/FileCacheStorage.php +++ b/src/Caching/ValueObject/Storage/FileCacheStorage.php @@ -6,7 +6,6 @@ use FilesystemIterator; use Nette\Utils\FileSystem; -use Nette\Utils\Random; use Rector\Caching\Contract\ValueObject\Storage\CacheStorageInterface; use Rector\Caching\ValueObject\CacheFilePaths; use Rector\Caching\ValueObject\CacheItem; @@ -55,7 +54,6 @@ public function save(string $key, string $variableKey, mixed $data): void $filePath = $cacheFilePaths->getFilePath(); - $tmpPath = \sprintf('%s/%s.tmp', $this->directory, Random::generate()); $errorBefore = \error_get_last(); $exported = @\var_export(new CacheItem($variableKey, $data), true); $errorAfter = \error_get_last(); @@ -68,18 +66,9 @@ public function save(string $key, string $variableKey, mixed $data): void )); } - // for performance reasons we don't use SmartFileSystem - FileSystem::write($tmpPath, \sprintf("assertDirectoryDoesNotExist(__DIR__ . '/Source/0e'); } + public function testSaveLeavesConcurrentReaderOnCompleteFile(): void + { + $filePath = __DIR__ . '/Source/0e/76/0e76658526655756207688271159624026011393.php'; + + $this->fileCacheStorage->save('aaK1STfY', 'TEST', 'first'); + $contentsBeforeSave = (string) file_get_contents($filePath); + + // every parallel worker require()s this path while booting; open it as such a worker would, + // then save over it mid-read - an atomic write must leave the reader on the file it opened + $readerHandle = fopen($filePath, 'r'); + $this->assertNotFalse($readerHandle); + + $this->fileCacheStorage->save('aaK1STfY', 'TEST', 'second'); + + $contentsSeenByReader = stream_get_contents($readerHandle); + fclose($readerHandle); + + $this->assertSame($contentsBeforeSave, $contentsSeenByReader); + $this->assertSame('second', $this->fileCacheStorage->load('aaK1STfY', 'TEST')); + + $this->fileCacheStorage->clean('aaK1STfY'); + } + public function provideConfigFilePath(): string { return __DIR__ . '/config.php'; From c7d8a008194e4864629f482bdace46eb2c1e30a3 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 29 Sep 2026 14:14:58 +0200 Subject: [PATCH 2/4] add File Cache Storage Test workflow on ubuntu + windows --- .../workflows/file_cache_storage_test.yaml | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 .github/workflows/file_cache_storage_test.yaml diff --git a/.github/workflows/file_cache_storage_test.yaml b/.github/workflows/file_cache_storage_test.yaml new file mode 100644 index 00000000000..da790fc2b48 --- /dev/null +++ b/.github/workflows/file_cache_storage_test.yaml @@ -0,0 +1,39 @@ +name: File Cache Storage Test + +on: + pull_request: null + push: + branches: + - main + + + +env: + # see https://github.com/composer/composer/issues/9368#issuecomment-718112361 + COMPOSER_ROOT_VERSION: "dev-main" + +jobs: + tests: + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + php-versions: ['8.4'] + + runs-on: ${{ matrix.os }} + timeout-minutes: 3 + + name: PHP ${{ matrix.php-versions }} tests (${{ matrix.os }}) + steps: + - uses: actions/checkout@v5 + + - + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-versions }} + coverage: none + ini-values: zend.assertions=1 + + - uses: "ramsey/composer-install@v4" + + - run: vendor/bin/phpunit tests/Caching/ValueObject/Storage/FileCacheStorageTest.php --colors From 58779565652c2426732d0e1cc6f3496efdbd9a51 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 29 Sep 2026 14:29:53 +0200 Subject: [PATCH 3/4] skip POSIX-only atomic replace assertion on Windows --- tests/Caching/ValueObject/Storage/FileCacheStorageTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Caching/ValueObject/Storage/FileCacheStorageTest.php b/tests/Caching/ValueObject/Storage/FileCacheStorageTest.php index fc640f18e49..3eb792855cb 100644 --- a/tests/Caching/ValueObject/Storage/FileCacheStorageTest.php +++ b/tests/Caching/ValueObject/Storage/FileCacheStorageTest.php @@ -51,6 +51,12 @@ public function testClean(): void public function testSaveLeavesConcurrentReaderOnCompleteFile(): void { + if (\DIRECTORY_SEPARATOR === '\\') { + // Windows blocks rename() over a file open in another handle, so the atomic-replace-under-open-reader + // scenario this asserts is POSIX-only; on Windows writeAtomic() retries the transient lock instead + $this->markTestSkipped('Atomic replace under an open reader is POSIX-only'); + } + $filePath = __DIR__ . '/Source/0e/76/0e76658526655756207688271159624026011393.php'; $this->fileCacheStorage->save('aaK1STfY', 'TEST', 'first'); From 7f72ef290164e0b57f8be54952ef14351ea17069 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 29 Sep 2026 14:30:15 +0200 Subject: [PATCH 4/4] rename job to unique name for required check --- .github/workflows/file_cache_storage_test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/file_cache_storage_test.yaml b/.github/workflows/file_cache_storage_test.yaml index da790fc2b48..1fb817eeacf 100644 --- a/.github/workflows/file_cache_storage_test.yaml +++ b/.github/workflows/file_cache_storage_test.yaml @@ -23,7 +23,7 @@ jobs: runs-on: ${{ matrix.os }} timeout-minutes: 3 - name: PHP ${{ matrix.php-versions }} tests (${{ matrix.os }}) + name: File Cache Storage ${{ matrix.php-versions }} (${{ matrix.os }}) steps: - uses: actions/checkout@v5