From 59d320f38c3e501ccab2b803f3e8d8537e6dc032 Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 00:36:28 +0200 Subject: [PATCH 01/10] Make raster cache locks configurable and add safe retention --- CHANGELOG.md | 9 ++ README.md | 37 ++++++ classes/Base.php | 25 ++-- classes/CachePruner.php | 135 +++++++++++++++++++ classes/FileLock.php | 81 ++++++++++++ tests/CacheLockTest.php | 191 +++++++++++++++++++++++++++ tests/CachePrunerTest.php | 125 ++++++++++++++++++ tests/fixtures/cache-lock-worker.php | 25 ++++ 8 files changed, 612 insertions(+), 16 deletions(-) create mode 100644 classes/CachePruner.php create mode 100644 classes/FileLock.php create mode 100644 tests/CacheLockTest.php create mode 100644 tests/CachePrunerTest.php create mode 100644 tests/fixtures/cache-lock-worker.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 81ac1da..e5fbf1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `cacheLockTimeout` to configure the raster cache lock wait independently of upstream HTTP timeouts. +- `CachePruner::prune()` for scheduled raster tile retention, including obsolete namespaces and unused metadata. + +### Fixed + +- Close cache lock handles on successful and failed requests, and retry when a cleaner replaced the metadata inode. + ## [2.4.1] - 2026-08-28 ### Fixed diff --git a/README.md b/README.md index 69bd3ae..452fa95 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,8 @@ sending output. The configuration defines the behavior of the proxy. * `cacheServerPath`: Base directory for caching tiles and map assets. +* `cacheLockTimeout`: (Raster tiles, optional) Maximum seconds to wait for a tile's exclusive cache lock. + Defaults to `2.85` for compatibility; set it above the full upstream fetch and processing budget. * `ops`: (Raster tiles) Operation pipeline for bitmap tile requests. * `mapAssetBasePath`: (Mapbox styles) URL path prefix for proxied style JSON, vector tiles, sprites, and glyphs. * `styles`: (Mapbox styles) Named style configurations for `MapboxStyleProxy`. @@ -135,6 +137,41 @@ requests. HTTP(S) fetches go through Guzzle; `file://` URLs are read from disk. For tile pipelines, set `upstreamHttp` at the root of the config. A `src` operation can override it with its own `upstreamHttp` block. +### Raster cache retention + +`cacheServerTtl` controls freshness: an expired tile is refreshed on its next request, and a failed refresh can +still serve its cached copy. It does not evict unused files. + +Call `CachePruner::prune()` from cron or your application's scheduler to remove raster tile files older than a +separate retention period. This includes obsolete `cacheServerName` namespaces, old metadata without retained +tiles, and empty directories. The return value counts deleted files. For example, run this daily: + +```php +use OpenMapsight\TileProxy\CachePruner; + +$deleted = CachePruner::prune('/var/cache/mapsight-tile-proxy', 30 * 86400); +``` + +Choose a retention period longer than your raster `cacheServerTtl` values and any upstream minimum caching +requirements. Age is measured from tile writes, not filesystem atime; metadata tracks the last completed request. +Retention limits the age of stored tiles, not total disk usage. Mapbox style assets are outside this raster cleanup. + +The pruner takes the same per-tile lock as `Base`, skips busy tiles without waiting, and rechecks timestamps under +the lock. Waiting requests reopen metadata that was removed by the pruner before using the cache. Run cleanup +only after all processes sharing the cache use this version's locking, and leave `yoloOnLockTimeout` disabled. +Use a POSIX filesystem supporting `flock()` across those processes. Cache directories are reserved for proxy files; +symlinks are not followed by the pruner. + +For an upstream timeout of ten seconds plus tile processing, set `cacheLockTimeout` to a larger budget, for example: + +```jsonc +"cacheLockTimeout": 15, +"upstreamHttp": { "timeout": 10 } +``` + +Zero means a single non-blocking lock attempt. Negative or non-finite timeouts are rejected. Locks are closed on +both successful requests and failures; the default remains 2.85 seconds for existing deployments. + ### Error logging Most failures are handled gracefully in HTTP responses, but are otherwise silent unless logging is enabled. diff --git a/classes/Base.php b/classes/Base.php index 67489f2..16d09d3 100644 --- a/classes/Base.php +++ b/classes/Base.php @@ -45,23 +45,13 @@ public static function handleTileRequest( $cachePath = static::getCachePath($cfg, $reqArgs); $metadataPath = $cachePath . '-.metadata'; - Utils::mkdirp(dirname($cachePath)); - $lockH = fopen($metadataPath, 'cb'); - for ($lockTry = 1; ; ++$lockTry) { - if (flock($lockH, LOCK_EX | LOCK_NB) === true) { - break; - } - - if (10 <= $lockTry) { - if (isset($cfg['yoloOnLockTimeout']) && $cfg['yoloOnLockTimeout'] === true) { - @error_log('Yoloed lock for "' . $metadataPath . '"' . "\n"); - break; - } - + $lock = FileLock::acquire($metadataPath, (float)($cfg['cacheLockTimeout'] ?? 2.85)); + if ($lock === null) { + if (isset($cfg['yoloOnLockTimeout']) && $cfg['yoloOnLockTimeout'] === true) { + @error_log('Yoloed lock for "' . $metadataPath . '"' . "\n"); + } else { throw new RuntimeException('Can not lock "' . $metadataPath . '"'); } - - usleep($lockTry * $lockTry * 10 * 1000); } try { @@ -83,10 +73,13 @@ public static function handleTileRequest( $res->checkpointCache(); Metadata::save($meta); + if (!touch($metadataPath)) { + throw new RuntimeException('Could not update cache access time "' . $metadataPath . '"'); + } return static::buildTileResponse($res); } finally { - flock($lockH, LOCK_UN); + $lock?->release(); } } diff --git a/classes/CachePruner.php b/classes/CachePruner.php new file mode 100644 index 0000000..8cd56be --- /dev/null +++ b/classes/CachePruner.php @@ -0,0 +1,135 @@ +isLink()) { + continue; + } + + if ($file->isDir()) { + $deleted += self::pruneDirectory($file->getPathname(), $cutoff); + // Non-empty directories and directories in use are left in place. + @rmdir($file->getPathname()); + } + } + + $deleted += self::pruneDirectory($cacheServerPath, $cutoff); + return $deleted; + } + + private static function pruneDirectory(string $path, int $cutoff): int + { + try { + $files = new DirectoryIterator($path); + } catch (UnexpectedValueException $error) { + // Another pruner may have removed this empty directory during traversal. + if (!is_dir($path)) { + return 0; + } + throw $error; + } + + $groups = []; + foreach ($files as $file) { + if ($file->isLink() || !$file->isFile()) { + continue; + } + + if (preg_match('/^(\d+)-\.metadata$/D', $file->getFilename(), $matches) === 1) { + $groups[$matches[1]]['metadata'] = $file->getPathname(); + } elseif (preg_match('/^(\d+)-.+-\d+$/D', $file->getFilename(), $matches) === 1) { + $groups[$matches[1]]['tiles'][] = $file->getPathname(); + } + } + + $deleted = 0; + foreach ($groups as $group) { + if (isset($group['metadata'])) { + $deleted += self::pruneTile($group['metadata'], $group['tiles'] ?? [], $cutoff); + } + } + + return $deleted; + } + + /** @param list $tiles */ + private static function pruneTile(string $metadataPath, array $tiles, int $cutoff): int + { + $lock = FileLock::acquire($metadataPath, 0, false); + if ($lock === null) { + return 0; + } + + try { + $deleted = 0; + $hasTiles = false; + + foreach ($tiles as $tilePath) { + if (self::isExpired($tilePath, $cutoff)) { + self::delete($tilePath); + ++$deleted; + } else { + $hasTiles = true; + } + } + + // Base touches metadata under this lock on every completed request, including + // when a new cache namespace was created after the directory scan. + if (!$hasTiles && self::isExpired($metadataPath, $cutoff)) { + self::delete($metadataPath); + ++$deleted; + } + + return $deleted; + } finally { + $lock->release(); + } + } + + private static function isExpired(string $path, int $cutoff): bool + { + clearstatcache(true, $path); + if (is_link($path)) { + return false; + } + $mtime = @filemtime($path); + return $mtime !== false && $mtime < $cutoff; + } + + private static function delete(string $path): void + { + if (!@unlink($path)) { + throw new RuntimeException('Could not delete cache file "' . $path . '"'); + } + } +} diff --git a/classes/FileLock.php b/classes/FileLock.php new file mode 100644 index 0000000..a893ed8 --- /dev/null +++ b/classes/FileLock.php @@ -0,0 +1,81 @@ += $deadline) { + throw new RuntimeException('Could not open lock file "' . $path . '"'); + } + } else { + if (flock($handle, LOCK_EX | LOCK_NB) && self::isCurrentFile($handle, $path)) { + return new self($handle); + } + + fclose($handle); + } + + $remaining = $deadline - hrtime(true) / 1e9; + if ($remaining <= 0) { + return null; + } + + usleep((int)min(50000, ceil($remaining * 1e6))); + } while (true); + } + + /** @param resource $handle */ + private static function isCurrentFile($handle, string $path): bool + { + clearstatcache(true, $path); + $current = @stat($path); + $opened = fstat($handle); + + // A cleaner may unlink metadata while another request has its old inode open. + return $current !== false && $opened !== false + && $current['dev'] === $opened['dev'] && $current['ino'] === $opened['ino']; + } + + public function release(): void + { + if (is_resource($this->handle)) { + fclose($this->handle); + $this->handle = null; + } + } + + public function __destruct() + { + $this->release(); + } +} diff --git a/tests/CacheLockTest.php b/tests/CacheLockTest.php new file mode 100644 index 0000000..5416a0b --- /dev/null +++ b/tests/CacheLockTest.php @@ -0,0 +1,191 @@ +worker('hold', $this->metadataPath(), 0, 3100000); + $this->assertSame("LOCKED\n", fgets($worker['pipes'][1])); + + $response = Base::handleTileRequest($this->config(4)); + + $this->assertSame('tile', $response->body); + $this->assertSame('tile', file_get_contents($this->tempDir . '/cache/0/0/0-source-0')); + } + + public function testConfiguredTimeoutIsBoundedAndClosesItsHandle(): void + { + $lock = FileLock::acquire($this->metadataPath(), 0); + $this->assertNotNull($lock); + $start = hrtime(true); + try { + Base::handleTileRequest($this->config(0.03)); + $this->fail('A competing request must time out'); + } catch (RuntimeException $error) { + $this->assertStringContainsString('Can not lock', $error->getMessage()); + $this->assertLessThan(1, (hrtime(true) - $start) / 1e9); + } finally { + $lock->release(); + } + + $nextLock = FileLock::acquire($this->metadataPath(), 0); + $this->assertNotNull($nextLock); + $nextLock->release(); + } + + public function testPipelineFailureReleasesTheLock(): void + { + $cfg = $this->config(0); + $cfg['ops'][] = ['op' => 'unknown']; + try { + Base::handleTileRequest($cfg); + $this->fail('An unknown operation must fail'); + } catch (RuntimeException $error) { + $this->assertStringContainsString('No op handler', $error->getMessage()); + } + + $lock = FileLock::acquire($this->metadataPath(), 0); + $this->assertNotNull($lock); + $lock->release(); + } + + public function testWaiterCannotUseAnUnlinkedMetadataInode(): void + { + $worker = $this->worker('wait', $this->metadataPath(), 0.3); + $this->assertSame("READY\n", fgets($worker['pipes'][1])); + $oldLock = FileLock::acquire($this->metadataPath(), 0); + $replacement = FileLock::acquire($this->tempDir . '/replacement', 0); + $this->assertNotNull($oldLock); + $this->assertNotNull($replacement); + fwrite($worker['pipes'][0], "GO\n"); + usleep(100000); + + // Atomically replace the locked inode, as cleanup followed by a new request can do. + rename($this->tempDir . '/replacement', $this->metadataPath()); + $oldLock->release(); + $this->assertSame("TIMEOUT\n", fgets($worker['pipes'][1])); + // Wait for process exit before checking that a new request can acquire the lock. + $this->assertSame('', stream_get_contents($worker['pipes'][1])); + $replacement->release(); + + $this->assertNotNull(FileLock::acquire($this->metadataPath(), 0)); + } + + public function testZeroTimeoutDoesNotWaitAndMissingReadOnlyLockIsNotCreated(): void + { + $this->assertNull(FileLock::acquire($this->tempDir . '/missing', 0, false)); + $this->assertFileDoesNotExist($this->tempDir . '/missing'); + $lock = FileLock::acquire($this->metadataPath(), 0); + $this->assertNotNull($lock); + $this->assertNull(FileLock::acquire($this->metadataPath(), 0)); + $lock->release(); + } + + #[DataProvider('invalidTimeouts')] + public function testInvalidTimeoutIsRejected(float $timeout): void + { + $this->expectException(InvalidArgumentException::class); + FileLock::acquire($this->metadataPath(), $timeout); + } + + public static function invalidTimeouts(): array + { + return [[-1.0], [INF], [NAN]]; + } + + public function testCacheHitUpdatesMetadataAccessTimeWithoutRefreshingTile(): void + { + Base::handleTileRequest($this->config(0)); + $tilePath = $this->tempDir . '/cache/0/0/0-source-0'; + $oldTime = time() - 60; + touch($tilePath, $oldTime); + touch($this->metadataPath(), $oldTime); + unlink($this->tempDir . '/tile'); + clearstatcache(); + + $this->assertSame('tile', Base::handleTileRequest($this->config(0))->body); + clearstatcache(); + $this->assertSame($oldTime, filemtime($tilePath)); + $this->assertGreaterThan($oldTime, filemtime($this->metadataPath())); + } + + private function config(float $timeout): array + { + return [ + 'cacheServerPath' => $this->tempDir . '/cache', + 'cacheLockTimeout' => $timeout, + 'ops' => [[ + 'cacheServerName' => 'source', + 'urls' => ['file://' . $this->tempDir . '/tile'], + 'mimeType' => 'image/png', + 'cacheBrowserTtl' => 60, + 'cacheServerTtl' => 3600, + ]], + ]; + } + + private function metadataPath(): string + { + return $this->tempDir . '/cache/0/0/0-.metadata'; + } + + private function worker(string $mode, string $path, float $timeout, int $holdMicroseconds = 0): array + { + $process = proc_open( + [PHP_BINARY, __DIR__ . '/fixtures/cache-lock-worker.php', $mode, $path, (string)$timeout, (string)$holdMicroseconds], + [['pipe', 'r'], ['pipe', 'w'], ['pipe', 'w']], + $pipes + ); + $this->assertIsResource($process); + stream_set_timeout($pipes[1], 10); + $worker = ['process' => $process, 'pipes' => $pipes]; + $this->workers[] = $worker; + return $worker; + } + + protected function setUp(): void + { + $this->tempDir = sys_get_temp_dir() . '/tile_proxy_lock_' . bin2hex(random_bytes(8)); + mkdir($this->tempDir); + file_put_contents($this->tempDir . '/tile', 'tile'); + $this->savedGet = $_GET; + $_GET = ['z' => '0', 'x' => '0', 'y' => '0']; + } + + protected function tearDown(): void + { + foreach ($this->workers as $worker) { + fclose($worker['pipes'][0]); + $errors = stream_get_contents($worker['pipes'][2]); + fclose($worker['pipes'][1]); + fclose($worker['pipes'][2]); + $status = proc_close($worker['process']); + $this->assertSame(0, $status, $errors); + } + $_GET = $this->savedGet; + $files = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($this->tempDir, RecursiveDirectoryIterator::SKIP_DOTS), + RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ($files as $file) { + $file->isDir() ? rmdir($file->getPathname()) : unlink($file->getPathname()); + } + rmdir($this->tempDir); + } +} diff --git a/tests/CachePrunerTest.php b/tests/CachePrunerTest.php new file mode 100644 index 0000000..34f7c85 --- /dev/null +++ b/tests/CachePrunerTest.php @@ -0,0 +1,125 @@ +write('dark/12/2048/1024-source-0', 120); + $this->write('dark/12/2048/1024-overlay-0', 120); + $this->write('dark/12/2048/1024-source-1', 120); + $this->write('dark/12/2048/1024-.metadata', 120); + + $this->assertSame(4, CachePruner::prune($this->tempDir, 60)); + $this->assertDirectoryDoesNotExist($this->tempDir . '/dark'); + $this->assertDirectoryExists($this->tempDir); + $this->assertSame(0, CachePruner::prune($this->tempDir, 60)); + } + + public function testRemovesObsoleteNamespacesButKeepsFreshTileAndItsMetadata(): void + { + $old = $this->write('1/0/0-old-url-hash-0', 120); + $fresh = $this->write('1/0/0-current-url-hash-0'); + $metadata = $this->write('1/0/0-.metadata', 120); + + $this->assertSame(1, CachePruner::prune($this->tempDir, 60)); + $this->assertFileDoesNotExist($old); + $this->assertFileExists($fresh); + $this->assertFileExists($metadata); + } + + public function testSkipsTilesLockedByARequestThenPrunesAfterRelease(): void + { + $tile = $this->write('1/0/0-source-0', 120); + $metadata = $this->write('1/0/0-.metadata', 120); + $lock = FileLock::acquire($metadata, 0); + $this->assertNotNull($lock); + + $this->assertSame(0, CachePruner::prune($this->tempDir, 60)); + $this->assertFileExists($tile); + $lock->release(); + $this->assertSame(2, CachePruner::prune($this->tempDir, 60)); + } + + public function testKeepsRecentlyRequestedMetadataEvenWhenAllTilesAreOld(): void + { + $this->write('1/0/0-source-0', 120); + $metadata = $this->write('1/0/0-.metadata'); + $this->write('1/0/1-.metadata', 120); + + $this->assertSame(2, CachePruner::prune($this->tempDir, 60)); + $this->assertFileExists($metadata); + } + + public function testIgnoresSymlinksUnrelatedFilesAndOtherTileCoordinates(): void + { + $this->write('1/0/1-.metadata', 120); + $this->write('1/0/1-source-0', 120); + $other = $this->write('1/0/10-source-0'); + $this->write('1/0/10-.metadata'); + $unrelated = $this->write('1/0/notes.txt', 120); + $asset = $this->write('map-assets/abc.json', 120); + $target = $this->write('target', 120); + $this->write('1/0/2-.metadata', 120); + symlink($target, $this->tempDir . '/1/0/2-source-0'); + symlink($this->tempDir . '/1', $this->tempDir . '/linked'); + + $this->assertSame(3, CachePruner::prune($this->tempDir, 60)); + foreach ([$other, $unrelated, $asset, $target, $this->tempDir . '/1/0/2-source-0'] as $path) { + $this->assertFileExists($path); + } + $this->assertTrue(is_link($this->tempDir . '/linked')); + } + + public function testMissingCacheIsANoOp(): void + { + $this->assertSame(0, CachePruner::prune($this->tempDir . '/missing', 60)); + $this->assertDirectoryDoesNotExist($this->tempDir . '/missing'); + } + + public function testNonPositiveRetentionIsRejected(): void + { + $this->expectException(InvalidArgumentException::class); + CachePruner::prune($this->tempDir, 0); + } + + private function write(string $relativePath, int $age = 0): string + { + $path = $this->tempDir . '/' . $relativePath; + if (!is_dir(dirname($path))) { + mkdir(dirname($path), 0777, true); + } + file_put_contents($path, '{}'); + touch($path, time() - $age); + return $path; + } + + protected function setUp(): void + { + $this->tempDir = sys_get_temp_dir() . '/tile_proxy_pruner_' . bin2hex(random_bytes(8)); + mkdir($this->tempDir); + } + + protected function tearDown(): void + { + $files = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($this->tempDir, RecursiveDirectoryIterator::SKIP_DOTS), + RecursiveIteratorIterator::CHILD_FIRST + ); + foreach ($files as $file) { + $file->isDir() && !$file->isLink() ? rmdir($file->getPathname()) : unlink($file->getPathname()); + } + rmdir($this->tempDir); + } +} diff --git a/tests/fixtures/cache-lock-worker.php b/tests/fixtures/cache-lock-worker.php new file mode 100644 index 0000000..d2d1883 --- /dev/null +++ b/tests/fixtures/cache-lock-worker.php @@ -0,0 +1,25 @@ +release(); +} From dbeeeb6873855a97d2c17a2cd60bb56fae2b2b11 Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 00:59:20 +0200 Subject: [PATCH 02/10] Preserve metadata for checkpoints missed by the pruning scan --- classes/CachePruner.php | 18 +++++++++-- tests/CachePrunerTest.php | 63 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/classes/CachePruner.php b/classes/CachePruner.php index 8cd56be..12f65d8 100644 --- a/classes/CachePruner.php +++ b/classes/CachePruner.php @@ -103,9 +103,9 @@ private static function pruneTile(string $metadataPath, array $tiles, int $cutof } } - // Base touches metadata under this lock on every completed request, including - // when a new cache namespace was created after the directory scan. - if (!$hasTiles && self::isExpired($metadataPath, $cutoff)) { + // A failed request can checkpoint a new namespace after the scan without touching + // metadata. Recheck the directory under the lock before removing its lock file. + if (!$hasTiles && self::isExpired($metadataPath, $cutoff) && !self::hasTiles($metadataPath)) { self::delete($metadataPath); ++$deleted; } @@ -116,6 +116,18 @@ private static function pruneTile(string $metadataPath, array $tiles, int $cutof } } + private static function hasTiles(string $metadataPath): bool + { + $tilePattern = '/^' . preg_quote(basename($metadataPath, '-.metadata'), '/') . '-.+-\d+$/D'; + foreach (new DirectoryIterator(dirname($metadataPath)) as $file) { + if (!$file->isLink() && $file->isFile() && preg_match($tilePattern, $file->getFilename()) === 1) { + return true; + } + } + + return false; + } + private static function isExpired(string $path, int $cutoff): bool { clearstatcache(true, $path); diff --git a/tests/CachePrunerTest.php b/tests/CachePrunerTest.php index 34f7c85..530c8e1 100644 --- a/tests/CachePrunerTest.php +++ b/tests/CachePrunerTest.php @@ -4,11 +4,15 @@ namespace OpenMapsight\TileProxy\Tests; use InvalidArgumentException; +use OpenMapsight\TileProxy\Base; use OpenMapsight\TileProxy\CachePruner; use OpenMapsight\TileProxy\FileLock; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; +use ReflectionMethod; +use RuntimeException; class CachePrunerTest extends TestCase { @@ -62,6 +66,65 @@ public function testKeepsRecentlyRequestedMetadataEvenWhenAllTilesAreOld(): void $this->assertFileExists($metadata); } + #[DataProvider('staleScans')] + public function testKeepsMetadataForCheckpointWrittenAfterScanByAFailedRequest(bool $scanHadTile): void + { + $metadata = $this->write('cache/1/0/0-.metadata', 120); + $scannedTiles = $scanHadTile ? [$this->write('cache/1/0/0-old-source-0', 120)] : []; + $source = $this->write('source'); + $metadataMtime = filemtime($metadata); + $savedGet = $_GET; + $_GET = ['z' => '1', 'x' => '0', 'y' => '0']; + + try { + Base::handleTileRequest([ + 'cacheServerPath' => $this->tempDir . '/cache', + 'cacheLockTimeout' => 0, + 'ops' => [ + [ + 'cacheServerName' => 'new-source', + 'urls' => ['file://' . $source], + 'mimeType' => 'image/png', + 'cacheBrowserTtl' => 60, + 'cacheServerTtl' => 3600, + ], + // Merge checkpoints the source before this invalid subpipeline fails. + ['op' => 'merge', 'ops' => [[]]], + ], + ]); + $this->fail('The merge subpipeline must fail after checkpointing the source'); + } catch (RuntimeException $error) { + $this->assertStringContainsString('Missing `cacheServerName`', $error->getMessage()); + } finally { + $_GET = $savedGet; + } + + $checkpoint = $this->tempDir . '/cache/1/0/0-new-source-0'; + $this->assertFileExists($checkpoint); + clearstatcache(true, $metadata); + $this->assertSame($metadataMtime, filemtime($metadata)); + + // Resume pruning with the pre-request scan to reproduce the interleaving without timing races. + $pruneTile = new ReflectionMethod(CachePruner::class, 'pruneTile'); + $this->assertSame($scanHadTile ? 1 : 0, $pruneTile->invoke(null, $metadata, $scannedTiles, time() - 60)); + $this->assertFileExists($checkpoint); + $this->assertFileExists($metadata); + + // Keeping metadata lets a later retention run remove the checkpoint once it ages out. + touch($checkpoint, time() - 120); + $this->assertSame(2, CachePruner::prune($this->tempDir, 60)); + $this->assertFileDoesNotExist($checkpoint); + $this->assertFileDoesNotExist($metadata); + } + + public static function staleScans(): array + { + return [ + 'metadata only' => [false], + 'obsolete namespace' => [true], + ]; + } + public function testIgnoresSymlinksUnrelatedFilesAndOtherTileCoordinates(): void { $this->write('1/0/1-.metadata', 120); From de566e9cc21c8c74cbb8fe64c8a862852f7b4deb Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 01:08:03 +0200 Subject: [PATCH 03/10] Retry cache directory races and filter pruning scans by filename --- CHANGELOG.md | 2 +- classes/CachePruner.php | 2 +- classes/FileLock.php | 10 ++++++++- tests/CacheLockTest.php | 48 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5fbf1a..67251fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Close cache lock handles on successful and failed requests, and retry when a cleaner replaced the metadata inode. +- Close cache lock handles on successful and failed requests, and retry when a cleaner replaces the metadata inode or removes an empty cache directory. ## [2.4.1] - 2026-08-28 diff --git a/classes/CachePruner.php b/classes/CachePruner.php index 12f65d8..0327542 100644 --- a/classes/CachePruner.php +++ b/classes/CachePruner.php @@ -120,7 +120,7 @@ private static function hasTiles(string $metadataPath): bool { $tilePattern = '/^' . preg_quote(basename($metadataPath, '-.metadata'), '/') . '-.+-\d+$/D'; foreach (new DirectoryIterator(dirname($metadataPath)) as $file) { - if (!$file->isLink() && $file->isFile() && preg_match($tilePattern, $file->getFilename()) === 1) { + if (preg_match($tilePattern, $file->getFilename()) === 1 && !$file->isLink() && $file->isFile()) { return true; } } diff --git a/classes/FileLock.php b/classes/FileLock.php index a893ed8..82a7e53 100644 --- a/classes/FileLock.php +++ b/classes/FileLock.php @@ -23,7 +23,15 @@ public static function acquire(string $path, float $timeout, bool $create = true $deadline = hrtime(true) / 1e9 + $timeout; do { if ($create) { - Utils::mkdirp(dirname($path)); + try { + Utils::mkdirp(dirname($path)); + } catch (RuntimeException $error) { + // A pruner can remove the directory between mkdirp()'s mkdir() and is_dir(). + // Let the open attempt below handle recovery within the same lock budget. + if (hrtime(true) / 1e9 >= $deadline) { + throw $error; + } + } } $handle = @fopen($path, $create ? 'c+b' : 'r+b'); diff --git a/tests/CacheLockTest.php b/tests/CacheLockTest.php index 5416a0b..d6484a3 100644 --- a/tests/CacheLockTest.php +++ b/tests/CacheLockTest.php @@ -97,6 +97,54 @@ public function testZeroTimeoutDoesNotWaitAndMissingReadOnlyLockIsNotCreated(): $lock->release(); } + public function testRetriesWhenDirectoryIsPrunedDuringCreation(): void + { + $path = $this->metadataPath(); + $directory = dirname($path); + mkdir($directory, 0777, true); + $removed = false; + set_error_handler(static function (int $severity, string $message) use ($directory, &$removed): bool { + if (!$removed && str_contains($message, 'mkdir(): File exists')) { + // Remove the empty directory between mkdir() failing and mkdirp() checking is_dir(). + rmdir($directory); + clearstatcache(true, $directory); + $removed = true; + return true; + } + + return false; + }); + + try { + $lock = FileLock::acquire($path, 0.2); + } finally { + restore_error_handler(); + } + + $this->assertTrue($removed); + $this->assertNotNull($lock); + $lock->release(); + $this->assertFileExists($path); + } + + public function testDirectoryCreationFailureIsReportedWithinTheLockBudget(): void + { + $parentFile = $this->tempDir . '/not-a-directory'; + file_put_contents($parentFile, 'blocked'); + $timeout = 0.03; + $start = hrtime(true); + + try { + FileLock::acquire($parentFile . '/metadata', $timeout); + $this->fail('A file cannot be used as the metadata directory'); + } catch (RuntimeException $error) { + $this->assertStringContainsString($parentFile, $error->getMessage()); + $elapsed = (hrtime(true) - $start) / 1e9; + $this->assertGreaterThanOrEqual($timeout, $elapsed); + $this->assertLessThan(1, $elapsed); + } + } + #[DataProvider('invalidTimeouts')] public function testInvalidTimeoutIsRejected(float $timeout): void { From 9c4cdf3dc4155f50c9b687e6b41d4e104b19a5c2 Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 01:15:02 +0200 Subject: [PATCH 04/10] Exclude shared Mapbox caches from raster pruning --- README.md | 3 +- classes/CachePruner.php | 9 +++++- tests/CachePrunerTest.php | 60 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 452fa95..f109eae 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,8 @@ $deleted = CachePruner::prune('/var/cache/mapsight-tile-proxy', 30 * 86400); Choose a retention period longer than your raster `cacheServerTtl` values and any upstream minimum caching requirements. Age is measured from tile writes, not filesystem atime; metadata tracks the last completed request. -Retention limits the age of stored tiles, not total disk usage. Mapbox style assets are outside this raster cleanup. +Retention limits the age of stored tiles, not total disk usage. The reserved `mapbox-style-proxy` subtree, including +its empty directories, is excluded from raster cleanup. The pruner takes the same per-tile lock as `Base`, skips busy tiles without waiting, and rechecks timestamps under the lock. Waiting requests reopen metadata that was removed by the pruner before using the cache. Run cleanup diff --git a/classes/CachePruner.php b/classes/CachePruner.php index 0327542..ba1f139 100644 --- a/classes/CachePruner.php +++ b/classes/CachePruner.php @@ -5,9 +5,11 @@ use DirectoryIterator; use InvalidArgumentException; +use RecursiveCallbackFilterIterator; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; use RuntimeException; +use SplFileInfo; use UnexpectedValueException; final class CachePruner @@ -25,8 +27,13 @@ public static function prune(string $cacheServerPath, int $maxAgeSeconds): int $cutoff = time() - $maxAgeSeconds; $deleted = 0; + // Mapbox asset writers do not share raster tile locks, even in the same cache root. + $mapboxCachePath = rtrim($cacheServerPath, '/') . '/mapbox-style-proxy'; $files = new RecursiveIteratorIterator( - new RecursiveDirectoryIterator($cacheServerPath, RecursiveDirectoryIterator::SKIP_DOTS), + new RecursiveCallbackFilterIterator( + new RecursiveDirectoryIterator($cacheServerPath, RecursiveDirectoryIterator::SKIP_DOTS), + static fn (SplFileInfo $file): bool => $file->getPathname() !== $mapboxCachePath + ), RecursiveIteratorIterator::CHILD_FIRST, RecursiveIteratorIterator::CATCH_GET_CHILD ); diff --git a/tests/CachePrunerTest.php b/tests/CachePrunerTest.php index 530c8e1..8340c8f 100644 --- a/tests/CachePrunerTest.php +++ b/tests/CachePrunerTest.php @@ -7,6 +7,7 @@ use OpenMapsight\TileProxy\Base; use OpenMapsight\TileProxy\CachePruner; use OpenMapsight\TileProxy\FileLock; +use OpenMapsight\TileProxy\MapboxStyleProxy; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use RecursiveDirectoryIterator; @@ -125,6 +126,65 @@ public static function staleScans(): array ]; } + public function testSharedCachePruningPreservesAMapboxAssetWriteInProgress(): void + { + $this->write('1/0/0-source-0', 120); + $this->write('1/0/0-.metadata', 120); + $style = '{"version":8,"sources":{},"layers":[]}'; + $upstreamStyle = $this->write('upstream-style.json'); + file_put_contents($upstreamStyle, $style); + $mapboxDirectory = $this->tempDir . '/mapbox-style-proxy/example/style'; + mkdir($mapboxDirectory, 0777, true); + $cacheRoot = $this->tempDir; + $pruned = false; + $deleted = null; + + set_error_handler(static function (int $severity, string $message) use ($cacheRoot, &$pruned, &$deleted): bool { + if (!$pruned && str_contains($message, 'mkdir(): File exists')) { + // Pause the asset writer in mkdirp(), while its existing directory is still empty. + $pruned = true; + $deleted = CachePruner::prune($cacheRoot, 60); + return true; + } + + return false; + }); + + try { + $response = MapboxStyleProxy::handleRequest([ + 'cacheServerPath' => $this->tempDir, + 'styles' => ['example' => [ + 'upstreamStyleUrl' => 'file://' . $upstreamStyle, + 'allowedSchemes' => ['file'], + 'allowedPathPrefixes' => [$this->tempDir . '/'], + ]], + ], '/styles/example.json'); + } finally { + restore_error_handler(); + } + + $this->assertTrue($pruned); + $this->assertSame(2, $deleted); + $this->assertSame(8, json_decode($response->body, true)['version']); + $this->assertDirectoryExists($mapboxDirectory); + $this->assertSame($style, file_get_contents($mapboxDirectory . '/' . sha1('file://' . $upstreamStyle))); + } + + public function testIgnoresFilesAndEmptyDirectoriesInReservedMapboxCacheTree(): void + { + $asset = $this->write('mapbox-style-proxy/example/style/abc', 120); + $tile = $this->write('mapbox-style-proxy/example/tile/1-source-0', 120); + $metadata = $this->write('mapbox-style-proxy/example/tile/1-.metadata', 120); + $emptyDirectory = $this->tempDir . '/mapbox-style-proxy/example/glyph'; + mkdir($emptyDirectory); + + $this->assertSame(0, CachePruner::prune($this->tempDir . '/', 60)); + foreach ([$asset, $tile, $metadata] as $path) { + $this->assertFileExists($path); + } + $this->assertDirectoryExists($emptyDirectory); + } + public function testIgnoresSymlinksUnrelatedFilesAndOtherTileCoordinates(): void { $this->write('1/0/1-.metadata', 120); From dfc058cb1467548880cc027f0f23cd204aad5df4 Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 01:22:54 +0200 Subject: [PATCH 05/10] Retry failed lock opens until the configured deadline --- classes/FileLock.php | 4 ++-- tests/CacheLockTest.php | 25 +++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/classes/FileLock.php b/classes/FileLock.php index 82a7e53..a43ba61 100644 --- a/classes/FileLock.php +++ b/classes/FileLock.php @@ -40,9 +40,9 @@ public static function acquire(string $path, float $timeout, bool $create = true return null; } - // An empty cache directory may have been pruned after mkdirp(). + // The parent may have been removed before fopen() and recreated before this check. clearstatcache(true, dirname($path)); - if (is_dir(dirname($path)) || hrtime(true) / 1e9 >= $deadline) { + if (hrtime(true) / 1e9 >= $deadline) { throw new RuntimeException('Could not open lock file "' . $path . '"'); } } else { diff --git a/tests/CacheLockTest.php b/tests/CacheLockTest.php index d6484a3..a4d721f 100644 --- a/tests/CacheLockTest.php +++ b/tests/CacheLockTest.php @@ -97,13 +97,17 @@ public function testZeroTimeoutDoesNotWaitAndMissingReadOnlyLockIsNotCreated(): $lock->release(); } - public function testRetriesWhenDirectoryIsPrunedDuringCreation(): void + #[DataProvider('directoryRecreation')] + public function testRetriesWhenDirectoryIsPrunedDuringCreation(bool $recreateAfterFailedOpen): void { $path = $this->metadataPath(); $directory = dirname($path); mkdir($directory, 0777, true); $removed = false; - set_error_handler(static function (int $severity, string $message) use ($directory, &$removed): bool { + $recreated = false; + set_error_handler(static function (int $severity, string $message) use ( + $directory, $recreateAfterFailedOpen, &$removed, &$recreated + ): bool { if (!$removed && str_contains($message, 'mkdir(): File exists')) { // Remove the empty directory between mkdir() failing and mkdirp() checking is_dir(). rmdir($directory); @@ -112,6 +116,14 @@ public function testRetriesWhenDirectoryIsPrunedDuringCreation(): void return true; } + if ($recreateAfterFailedOpen && $removed && !$recreated && str_contains($message, 'fopen(')) { + // Another request recreates the parent after the open fails, before its error is checked. + mkdir($directory, 0777, true); + clearstatcache(true, $directory); + $recreated = true; + return true; + } + return false; }); @@ -122,11 +134,20 @@ public function testRetriesWhenDirectoryIsPrunedDuringCreation(): void } $this->assertTrue($removed); + $this->assertSame($recreateAfterFailedOpen, $recreated); $this->assertNotNull($lock); $lock->release(); $this->assertFileExists($path); } + public static function directoryRecreation(): array + { + return [ + 'recreated on retry' => [false], + 'recreated after failed open' => [true], + ]; + } + public function testDirectoryCreationFailureIsReportedWithinTheLockBudget(): void { $parentFile = $this->tempDir . '/not-a-directory'; From f3443c3039249b7a999f48ddd6c5b51f6171bbe4 Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 01:29:31 +0200 Subject: [PATCH 06/10] Normalize cache roots before excluding Mapbox directories --- classes/CachePruner.php | 4 +++- tests/CachePrunerTest.php | 22 +++++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/classes/CachePruner.php b/classes/CachePruner.php index ba1f139..bb58fd0 100644 --- a/classes/CachePruner.php +++ b/classes/CachePruner.php @@ -25,10 +25,12 @@ public static function prune(string $cacheServerPath, int $maxAgeSeconds): int return 0; } + // Use the same separators for iterator paths and the reserved-subtree comparison. + $cacheServerPath = rtrim($cacheServerPath, '/') . '/'; $cutoff = time() - $maxAgeSeconds; $deleted = 0; // Mapbox asset writers do not share raster tile locks, even in the same cache root. - $mapboxCachePath = rtrim($cacheServerPath, '/') . '/mapbox-style-proxy'; + $mapboxCachePath = $cacheServerPath . 'mapbox-style-proxy'; $files = new RecursiveIteratorIterator( new RecursiveCallbackFilterIterator( new RecursiveDirectoryIterator($cacheServerPath, RecursiveDirectoryIterator::SKIP_DOTS), diff --git a/tests/CachePrunerTest.php b/tests/CachePrunerTest.php index 8340c8f..66b9477 100644 --- a/tests/CachePrunerTest.php +++ b/tests/CachePrunerTest.php @@ -126,7 +126,8 @@ public static function staleScans(): array ]; } - public function testSharedCachePruningPreservesAMapboxAssetWriteInProgress(): void + #[DataProvider('cacheRootSuffixes')] + public function testSharedCachePruningPreservesAMapboxAssetWriteInProgress(string $rootSuffix): void { $this->write('1/0/0-source-0', 120); $this->write('1/0/0-.metadata', 120); @@ -135,7 +136,7 @@ public function testSharedCachePruningPreservesAMapboxAssetWriteInProgress(): vo file_put_contents($upstreamStyle, $style); $mapboxDirectory = $this->tempDir . '/mapbox-style-proxy/example/style'; mkdir($mapboxDirectory, 0777, true); - $cacheRoot = $this->tempDir; + $cacheRoot = $this->tempDir . $rootSuffix; $pruned = false; $deleted = null; @@ -152,7 +153,7 @@ public function testSharedCachePruningPreservesAMapboxAssetWriteInProgress(): vo try { $response = MapboxStyleProxy::handleRequest([ - 'cacheServerPath' => $this->tempDir, + 'cacheServerPath' => $cacheRoot, 'styles' => ['example' => [ 'upstreamStyleUrl' => 'file://' . $upstreamStyle, 'allowedSchemes' => ['file'], @@ -170,7 +171,8 @@ public function testSharedCachePruningPreservesAMapboxAssetWriteInProgress(): vo $this->assertSame($style, file_get_contents($mapboxDirectory . '/' . sha1('file://' . $upstreamStyle))); } - public function testIgnoresFilesAndEmptyDirectoriesInReservedMapboxCacheTree(): void + #[DataProvider('cacheRootSuffixes')] + public function testIgnoresFilesAndEmptyDirectoriesInReservedMapboxCacheTree(string $rootSuffix): void { $asset = $this->write('mapbox-style-proxy/example/style/abc', 120); $tile = $this->write('mapbox-style-proxy/example/tile/1-source-0', 120); @@ -178,13 +180,23 @@ public function testIgnoresFilesAndEmptyDirectoriesInReservedMapboxCacheTree(): $emptyDirectory = $this->tempDir . '/mapbox-style-proxy/example/glyph'; mkdir($emptyDirectory); - $this->assertSame(0, CachePruner::prune($this->tempDir . '/', 60)); + $this->assertSame(0, CachePruner::prune($this->tempDir . $rootSuffix, 60)); foreach ([$asset, $tile, $metadata] as $path) { $this->assertFileExists($path); } $this->assertDirectoryExists($emptyDirectory); } + public static function cacheRootSuffixes(): array + { + return [ + 'no trailing slash' => [''], + 'one trailing slash' => ['/'], + 'two trailing slashes' => ['//'], + 'three trailing slashes' => ['///'], + ]; + } + public function testIgnoresSymlinksUnrelatedFilesAndOtherTileCoordinates(): void { $this->write('1/0/1-.metadata', 120); From 61a9902f5f1d65463538f1a8dd6964020eac2a8d Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 01:35:37 +0200 Subject: [PATCH 07/10] Prune tiles with empty cache namespaces safely --- classes/CachePruner.php | 4 ++-- tests/CachePrunerTest.php | 34 ++++++++++++++++++++++++---------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/classes/CachePruner.php b/classes/CachePruner.php index bb58fd0..f87934b 100644 --- a/classes/CachePruner.php +++ b/classes/CachePruner.php @@ -76,7 +76,7 @@ private static function pruneDirectory(string $path, int $cutoff): int if (preg_match('/^(\d+)-\.metadata$/D', $file->getFilename(), $matches) === 1) { $groups[$matches[1]]['metadata'] = $file->getPathname(); - } elseif (preg_match('/^(\d+)-.+-\d+$/D', $file->getFilename(), $matches) === 1) { + } elseif (preg_match('/^(\d+)-.*-\d+$/D', $file->getFilename(), $matches) === 1) { $groups[$matches[1]]['tiles'][] = $file->getPathname(); } } @@ -127,7 +127,7 @@ private static function pruneTile(string $metadataPath, array $tiles, int $cutof private static function hasTiles(string $metadataPath): bool { - $tilePattern = '/^' . preg_quote(basename($metadataPath, '-.metadata'), '/') . '-.+-\d+$/D'; + $tilePattern = '/^' . preg_quote(basename($metadataPath, '-.metadata'), '/') . '-.*-\d+$/D'; foreach (new DirectoryIterator(dirname($metadataPath)) as $file) { if (preg_match($tilePattern, $file->getFilename()) === 1 && !$file->isLink() && $file->isFile()) { return true; diff --git a/tests/CachePrunerTest.php b/tests/CachePrunerTest.php index 66b9477..c08dd87 100644 --- a/tests/CachePrunerTest.php +++ b/tests/CachePrunerTest.php @@ -19,11 +19,12 @@ class CachePrunerTest extends TestCase { private string $tempDir; - public function testRemovesOldTilesMetadataAndEmptyDirectoriesIncludingPrefixes(): void + #[DataProvider('cacheNamespaces')] + public function testRemovesOldTilesMetadataAndEmptyDirectoriesIncludingPrefixes(string $namespace): void { - $this->write('dark/12/2048/1024-source-0', 120); + $this->write('dark/12/2048/1024-' . $namespace . '-0', 120); $this->write('dark/12/2048/1024-overlay-0', 120); - $this->write('dark/12/2048/1024-source-1', 120); + $this->write('dark/12/2048/1024-' . $namespace . '-1', 120); $this->write('dark/12/2048/1024-.metadata', 120); $this->assertSame(4, CachePruner::prune($this->tempDir, 60)); @@ -32,10 +33,11 @@ public function testRemovesOldTilesMetadataAndEmptyDirectoriesIncludingPrefixes( $this->assertSame(0, CachePruner::prune($this->tempDir, 60)); } - public function testRemovesObsoleteNamespacesButKeepsFreshTileAndItsMetadata(): void + #[DataProvider('cacheNamespaces')] + public function testRemovesObsoleteNamespacesButKeepsFreshTileAndItsMetadata(string $namespace): void { $old = $this->write('1/0/0-old-url-hash-0', 120); - $fresh = $this->write('1/0/0-current-url-hash-0'); + $fresh = $this->write('1/0/0-' . $namespace . '-0'); $metadata = $this->write('1/0/0-.metadata', 120); $this->assertSame(1, CachePruner::prune($this->tempDir, 60)); @@ -44,6 +46,14 @@ public function testRemovesObsoleteNamespacesButKeepsFreshTileAndItsMetadata(): $this->assertFileExists($metadata); } + public static function cacheNamespaces(): array + { + return [ + 'named namespace' => ['current-url-hash'], + 'empty namespace' => [''], + ]; + } + public function testSkipsTilesLockedByARequestThenPrunesAfterRelease(): void { $tile = $this->write('1/0/0-source-0', 120); @@ -68,7 +78,9 @@ public function testKeepsRecentlyRequestedMetadataEvenWhenAllTilesAreOld(): void } #[DataProvider('staleScans')] - public function testKeepsMetadataForCheckpointWrittenAfterScanByAFailedRequest(bool $scanHadTile): void + public function testKeepsMetadataForCheckpointWrittenAfterScanByAFailedRequest( + bool $scanHadTile, string $checkpointNamespace + ): void { $metadata = $this->write('cache/1/0/0-.metadata', 120); $scannedTiles = $scanHadTile ? [$this->write('cache/1/0/0-old-source-0', 120)] : []; @@ -83,7 +95,7 @@ public function testKeepsMetadataForCheckpointWrittenAfterScanByAFailedRequest(b 'cacheLockTimeout' => 0, 'ops' => [ [ - 'cacheServerName' => 'new-source', + 'cacheServerName' => $checkpointNamespace, 'urls' => ['file://' . $source], 'mimeType' => 'image/png', 'cacheBrowserTtl' => 60, @@ -100,7 +112,7 @@ public function testKeepsMetadataForCheckpointWrittenAfterScanByAFailedRequest(b $_GET = $savedGet; } - $checkpoint = $this->tempDir . '/cache/1/0/0-new-source-0'; + $checkpoint = $this->tempDir . '/cache/1/0/0-' . $checkpointNamespace . '-0'; $this->assertFileExists($checkpoint); clearstatcache(true, $metadata); $this->assertSame($metadataMtime, filemtime($metadata)); @@ -121,8 +133,10 @@ public function testKeepsMetadataForCheckpointWrittenAfterScanByAFailedRequest(b public static function staleScans(): array { return [ - 'metadata only' => [false], - 'obsolete namespace' => [true], + 'metadata only' => [false, 'new-source'], + 'obsolete namespace' => [true, 'new-source'], + 'metadata only with empty checkpoint namespace' => [false, ''], + 'obsolete namespace with empty checkpoint namespace' => [true, ''], ]; } From 798068cbd97a4899afe7c54e406ef4a82400c050 Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 01:47:08 +0200 Subject: [PATCH 08/10] Refresh directory status when concurrent pruning removes it --- classes/CachePruner.php | 1 + tests/CachePrunerTest.php | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/classes/CachePruner.php b/classes/CachePruner.php index f87934b..257d0b2 100644 --- a/classes/CachePruner.php +++ b/classes/CachePruner.php @@ -62,6 +62,7 @@ private static function pruneDirectory(string $path, int $cutoff): int $files = new DirectoryIterator($path); } catch (UnexpectedValueException $error) { // Another pruner may have removed this empty directory during traversal. + clearstatcache(true, $path); if (!is_dir($path)) { return 0; } diff --git a/tests/CachePrunerTest.php b/tests/CachePrunerTest.php index c08dd87..c46d7ec 100644 --- a/tests/CachePrunerTest.php +++ b/tests/CachePrunerTest.php @@ -237,6 +237,63 @@ public function testMissingCacheIsANoOp(): void $this->assertDirectoryDoesNotExist($this->tempDir . '/missing'); } + public function testDirectoryRemovedByAnotherProcessIsANoOpDespiteCachedStat(): void + { + if (!defined('SIGSTOP') || !defined('SIGCONT')) { + $this->markTestSkipped('Process signals are required to preserve the parent stat cache'); + } + + $directory = $this->tempDir . '/removed'; + mkdir($directory); + $pruneDirectory = new ReflectionMethod(CachePruner::class, 'pruneDirectory'); + + // A subprocess removes the directory without clearing this process's positive stat cache. + $process = proc_open( + [PHP_BINARY, '-r', 'fgets(STDIN); exit(rmdir($argv[1]) ? 0 : 1);', $directory], + [['pipe', 'r'], ['pipe', 'w'], ['pipe', 'w']], + $pipes + ); + $this->assertIsResource($process); + + try { + proc_terminate($process, SIGSTOP); + $deadline = hrtime(true) + 2e9; + do { + $status = proc_get_status($process); + if ($status['stopped'] || !$status['running'] || hrtime(true) >= $deadline) { + break; + } + usleep(1000); + } while (true); + $this->assertTrue($status['stopped']); + fwrite($pipes[0], "GO\n"); + $this->assertTrue(is_dir($directory)); + + // Signals and process-status polling avoid pipe I/O, which clears PHP's stat cache. + proc_terminate($process, SIGCONT); + $deadline = hrtime(true) + 2e9; + do { + $status = proc_get_status($process); + if (!$status['running'] || hrtime(true) >= $deadline) { + break; + } + usleep(1000); + } while (true); + $this->assertTrue(is_dir($directory), 'The parent must retain the stale positive directory stat'); + + $this->assertSame(0, $pruneDirectory->invoke(null, $directory, time() - 60)); + $this->assertFalse($status['running']); + $this->assertSame(0, $status['exitcode']); + $this->assertDirectoryDoesNotExist($directory); + } finally { + proc_terminate($process, SIGCONT); + fclose($pipes[0]); + fclose($pipes[1]); + fclose($pipes[2]); + proc_close($process); + } + } + public function testNonPositiveRetentionIsRejected(): void { $this->expectException(InvalidArgumentException::class); From f31af336f86889ece5936efd40fe950a6d76de93 Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 01:51:46 +0200 Subject: [PATCH 09/10] Handle cache root removal during pruning initialization --- classes/CachePruner.php | 12 +++++++++++- tests/CachePrunerTest.php | 16 ++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/classes/CachePruner.php b/classes/CachePruner.php index 257d0b2..fa39127 100644 --- a/classes/CachePruner.php +++ b/classes/CachePruner.php @@ -31,9 +31,19 @@ public static function prune(string $cacheServerPath, int $maxAgeSeconds): int $deleted = 0; // Mapbox asset writers do not share raster tile locks, even in the same cache root. $mapboxCachePath = $cacheServerPath . 'mapbox-style-proxy'; + try { + $directories = new RecursiveDirectoryIterator($cacheServerPath, RecursiveDirectoryIterator::SKIP_DOTS); + } catch (UnexpectedValueException $error) { + clearstatcache(true, $cacheServerPath); + if (!is_dir($cacheServerPath)) { + return 0; + } + throw $error; + } + $files = new RecursiveIteratorIterator( new RecursiveCallbackFilterIterator( - new RecursiveDirectoryIterator($cacheServerPath, RecursiveDirectoryIterator::SKIP_DOTS), + $directories, static fn (SplFileInfo $file): bool => $file->getPathname() !== $mapboxCachePath ), RecursiveIteratorIterator::CHILD_FIRST, diff --git a/tests/CachePrunerTest.php b/tests/CachePrunerTest.php index c46d7ec..3a55134 100644 --- a/tests/CachePrunerTest.php +++ b/tests/CachePrunerTest.php @@ -237,7 +237,8 @@ public function testMissingCacheIsANoOp(): void $this->assertDirectoryDoesNotExist($this->tempDir . '/missing'); } - public function testDirectoryRemovedByAnotherProcessIsANoOpDespiteCachedStat(): void + #[DataProvider('pruneEntryPoints')] + public function testDirectoryRemovedByAnotherProcessIsANoOpDespiteCachedStat(bool $pruneRoot): void { if (!defined('SIGSTOP') || !defined('SIGCONT')) { $this->markTestSkipped('Process signals are required to preserve the parent stat cache'); @@ -281,7 +282,10 @@ public function testDirectoryRemovedByAnotherProcessIsANoOpDespiteCachedStat(): } while (true); $this->assertTrue(is_dir($directory), 'The parent must retain the stale positive directory stat'); - $this->assertSame(0, $pruneDirectory->invoke(null, $directory, time() - 60)); + $deleted = $pruneRoot + ? CachePruner::prune($directory, 60) + : $pruneDirectory->invoke(null, $directory, time() - 60); + $this->assertSame(0, $deleted); $this->assertFalse($status['running']); $this->assertSame(0, $status['exitcode']); $this->assertDirectoryDoesNotExist($directory); @@ -294,6 +298,14 @@ public function testDirectoryRemovedByAnotherProcessIsANoOpDespiteCachedStat(): } } + public static function pruneEntryPoints(): array + { + return [ + 'directory sweep' => [false], + 'cache root' => [true], + ]; + } + public function testNonPositiveRetentionIsRejected(): void { $this->expectException(InvalidArgumentException::class); From 2ec29cc2317ff9f4cf60c3eb3e66d161ed6da085 Mon Sep 17 00:00:00 2001 From: Paul Golmann Date: Thu, 8 Oct 2026 01:57:55 +0200 Subject: [PATCH 10/10] Retry pruning directory opens after concurrent recreation --- classes/CachePruner.php | 51 ++++++++++++++++++++++++++------------- tests/CachePrunerTest.php | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 17 deletions(-) diff --git a/classes/CachePruner.php b/classes/CachePruner.php index fa39127..eea2e80 100644 --- a/classes/CachePruner.php +++ b/classes/CachePruner.php @@ -31,14 +31,14 @@ public static function prune(string $cacheServerPath, int $maxAgeSeconds): int $deleted = 0; // Mapbox asset writers do not share raster tile locks, even in the same cache root. $mapboxCachePath = $cacheServerPath . 'mapbox-style-proxy'; - try { - $directories = new RecursiveDirectoryIterator($cacheServerPath, RecursiveDirectoryIterator::SKIP_DOTS); - } catch (UnexpectedValueException $error) { - clearstatcache(true, $cacheServerPath); - if (!is_dir($cacheServerPath)) { - return 0; - } - throw $error; + $directories = self::openDirectory( + $cacheServerPath, + static fn (): RecursiveDirectoryIterator => new RecursiveDirectoryIterator( + $cacheServerPath, RecursiveDirectoryIterator::SKIP_DOTS + ) + ); + if ($directories === null) { + return 0; } $files = new RecursiveIteratorIterator( @@ -68,15 +68,9 @@ public static function prune(string $cacheServerPath, int $maxAgeSeconds): int private static function pruneDirectory(string $path, int $cutoff): int { - try { - $files = new DirectoryIterator($path); - } catch (UnexpectedValueException $error) { - // Another pruner may have removed this empty directory during traversal. - clearstatcache(true, $path); - if (!is_dir($path)) { - return 0; - } - throw $error; + $files = self::openDirectory($path, static fn (): DirectoryIterator => new DirectoryIterator($path)); + if ($files === null) { + return 0; } $groups = []; @@ -102,6 +96,29 @@ private static function pruneDirectory(string $path, int $cutoff): int return $deleted; } + /** + * @template T of DirectoryIterator + * @param callable(): T $open + * @return T|null + */ + private static function openDirectory(string $path, callable $open): ?DirectoryIterator + { + for ($attempt = 0; ; ++$attempt) { + try { + return $open(); + } catch (UnexpectedValueException $error) { + clearstatcache(true, $path); + if (!is_dir($path)) { + return null; + } + // A request may have recreated the directory after the failed open. Retry once. + if ($attempt > 0) { + throw $error; + } + } + } + } + /** @param list $tiles */ private static function pruneTile(string $metadataPath, array $tiles, int $cutoff): int { diff --git a/tests/CachePrunerTest.php b/tests/CachePrunerTest.php index 3a55134..bcf589b 100644 --- a/tests/CachePrunerTest.php +++ b/tests/CachePrunerTest.php @@ -3,6 +3,7 @@ namespace OpenMapsight\TileProxy\Tests; +use DirectoryIterator; use InvalidArgumentException; use OpenMapsight\TileProxy\Base; use OpenMapsight\TileProxy\CachePruner; @@ -14,6 +15,7 @@ use RecursiveIteratorIterator; use ReflectionMethod; use RuntimeException; +use UnexpectedValueException; class CachePrunerTest extends TestCase { @@ -306,6 +308,50 @@ public static function pruneEntryPoints(): array ]; } + #[DataProvider('directoryIteratorClasses')] + public function testRetriesOpeningADirectoryRecreatedAfterTheFailedOpen(string $iteratorClass): void + { + $directory = $this->tempDir . '/recreated'; + $recreated = false; + $open = static function () use ($directory, $iteratorClass, &$recreated): DirectoryIterator { + try { + return new $iteratorClass($directory); + } catch (UnexpectedValueException $error) { + // A request restores the directory between its failed open and the pruner's check. + mkdir($directory); + $recreated = true; + throw $error; + } + }; + + $openDirectory = new ReflectionMethod(CachePruner::class, 'openDirectory'); + $iterator = $openDirectory->invoke(null, $directory, $open); + + $this->assertTrue($recreated); + $this->assertInstanceOf($iteratorClass, $iterator); + $this->assertDirectoryExists($directory); + } + + public static function directoryIteratorClasses(): array + { + return [ + 'directory sweep' => [DirectoryIterator::class], + 'cache root' => [RecursiveDirectoryIterator::class], + ]; + } + + public function testPersistentDirectoryOpenFailuresAreReported(): void + { + $open = static function (): DirectoryIterator { + throw new UnexpectedValueException('Persistent directory open failure'); + }; + $openDirectory = new ReflectionMethod(CachePruner::class, 'openDirectory'); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Persistent directory open failure'); + $openDirectory->invoke(null, $this->tempDir, $open); + } + public function testNonPositiveRetentionIsRejected(): void { $this->expectException(InvalidArgumentException::class);