From 664736bc2cef0a517d2a160d362cb91acecf62e3 Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Wed, 1 Jul 2026 11:13:21 +0530 Subject: [PATCH] fix: defer cache purge until the outermost transaction commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updateDocument()/deleteDocument() already purged the document cache a second time "after commit" so a concurrent reader could not re-cache the pre-commit version. That guard only held when the write owned the outermost transaction. When the call was nested inside a caller's withTransaction(), the "after commit" purge ran while inTransaction was still > 0 — before the real COMMIT — so any concurrent reader could read the old committed row and re-populate the cache, leaving stale data after the outer transaction finally committed. Queue every purge issued inside a transaction and replay it once the outermost frame commits, so the invalidation always lands after the rows are visible. The queue lives on the adapter because transaction depth and the cache are shared there across Database wrappers (Mirror shares its source's adapter and cache), and is dropped on full rollback. --- src/Database/Adapter.php | 38 ++++++++++++++++++++++++++++++++++++ src/Database/Adapter/SQL.php | 1 + src/Database/Database.php | 38 ++++++++++++++++++++++++++++++------ 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/Database/Adapter.php b/src/Database/Adapter.php index ef0598341..374eb44f6 100644 --- a/src/Database/Adapter.php +++ b/src/Database/Adapter.php @@ -31,6 +31,18 @@ abstract class Adapter protected int $inTransaction = 0; + /** + * Document cache keys purged while a transaction was open, keyed by + * "collectionId:id" for de-duplication. The Database flushes these once + * the outermost transaction commits so a reader that re-cached the + * pre-commit version in the meantime gets invalidated. Lives on the + * adapter because transaction depth and cache are shared here across + * Database wrappers (e.g. Mirror shares its source's adapter and cache). + * + * @var array + */ + protected array $purgeAfterCommit = []; + protected bool $alterLocks = false; protected bool $skipDuplicates = false; @@ -394,6 +406,32 @@ public function inTransaction(): bool return $this->inTransaction > 0; } + /** + * Queue a document cache key to be purged again once the outermost + * transaction commits. + * + * @param string $collectionId + * @param string $id + * @return void + */ + public function queueCachePurge(string $collectionId, string $id): void + { + $this->purgeAfterCommit[$collectionId . ':' . $id] = [$collectionId, $id]; + } + + /** + * Return and clear the queued post-commit cache purges. + * + * @return array + */ + public function dequeueCachePurges(): array + { + $purges = $this->purgeAfterCommit; + $this->purgeAfterCommit = []; + + return $purges; + } + /** * Run a callback with skipDuplicates enabled. * Duplicate key errors during createDocuments() will be silently skipped diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 88b739ca9..b45e846ad 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -184,6 +184,7 @@ public function reconnect(): void { $this->getPDO()->reconnect(); $this->inTransaction = 0; + $this->purgeAfterCommit = []; } /** diff --git a/src/Database/Database.php b/src/Database/Database.php index 74e75d883..7a58aa67c 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -1555,7 +1555,29 @@ public function getAdapter(): Adapter */ public function withTransaction(callable $callback): mixed { - return $this->adapter->withTransaction($callback); + try { + $result = $this->adapter->withTransaction($callback); + } catch (\Throwable $e) { + // Only the outermost frame owns the queue; nested frames leave it + // for the parent. On a full rollback nothing committed, so drop the + // queued purges rather than evicting cache for changes that reverted. + if (!$this->adapter->inTransaction()) { + $this->adapter->dequeueCachePurges(); + } + throw $e; + } + + // The rows are now committed and visible. Re-purge every key touched + // during the transaction so a reader that cached a pre-commit version + // in the meantime is invalidated. The queue lives on the (shared) + // adapter, so this covers writes routed through wrapper databases too. + if (!$this->adapter->inTransaction()) { + foreach ($this->adapter->dequeueCachePurges() as [$collectionId, $id]) { + $this->purgeCachedDocumentInternal($collectionId, $id); + } + } + + return $result; } /** @@ -6397,9 +6419,6 @@ public function updateDocument(string $collection, string $id, Document $documen return $document; } - // Purge again after commit so readers cannot re-cache the pre-commit version - $this->purgeCachedDocumentInternal($collection->getId(), $id); - if (!$this->inBatchRelationshipPopulation && $this->resolveRelationships) { $documents = $this->silent(fn () => $this->populateDocumentsRelationships([$document], $collection, $this->relationshipFetchDepth)); $document = $documents[0]; @@ -7786,8 +7805,6 @@ public function deleteDocument(string $collection, string $id): bool }); if ($deleted) { - // Purge again after commit so readers cannot re-cache the pre-commit version - $this->purgeCachedDocumentInternal($collection->getId(), $id); $this->trigger(self::EVENT_DOCUMENT_DELETE, $document); } @@ -8390,6 +8407,15 @@ protected function purgeCachedDocumentInternal(string $collectionId, ?string $id $this->cache->purge($collectionKey, $documentKey); $this->cache->purge($documentKey); + // A purge inside an open transaction targets rows that are not yet + // committed. A concurrent reader can re-populate the cache with the + // pre-commit version before this transaction commits, leaving stale + // data behind. Queue the key to purge again once the outermost + // transaction commits and the new rows are visible to everyone. + if ($this->adapter->inTransaction()) { + $this->adapter->queueCachePurge($collectionId, $id); + } + return true; }