From 7ff54c7c1212b05ed0393e0a92e96c6002d1dc51 Mon Sep 17 00:00:00 2001 From: Eric Eldredge Date: Wed, 19 Jul 2023 13:02:57 -0400 Subject: [PATCH] Refresh cache before writing contents to bundle (#9123) * Add Cache.refresh() method * Refresh cache before reading contents of bundle * Refresh cache before running WriteBundleRequest ..only if the packaging request didn't run in the main thread. * Document Cache.refresh() --------- Co-authored-by: David Alsh <12656294+alshdavid@users.noreply.github.com> --- packages/core/cache/src/FSCache.js | 4 ++++ packages/core/cache/src/IDBCache.browser.js | 4 ++++ packages/core/cache/src/LMDBCache.js | 8 ++++++++ packages/core/cache/src/types.js | 6 ++++++ .../core/core/src/requests/WriteBundlesRequest.js | 12 ++++++++++++ 5 files changed, 34 insertions(+) diff --git a/packages/core/cache/src/FSCache.js b/packages/core/cache/src/FSCache.js index 5529eebff79..ff7355a0096 100644 --- a/packages/core/cache/src/FSCache.js +++ b/packages/core/cache/src/FSCache.js @@ -116,6 +116,10 @@ export class FSCache implements Cache { logger.error(err, '@parcel/cache'); } } + + refresh(): void { + // NOOP + } } registerSerializableClass(`${packageJson.version}:FSCache`, FSCache); diff --git a/packages/core/cache/src/IDBCache.browser.js b/packages/core/cache/src/IDBCache.browser.js index 61e489fd6b3..15fbf65adbb 100644 --- a/packages/core/cache/src/IDBCache.browser.js +++ b/packages/core/cache/src/IDBCache.browser.js @@ -117,6 +117,10 @@ export class IDBCache implements Cache { setLargeBlob(key: string, contents: Buffer | string): Promise { return this.setBlob(key, contents); } + + refresh(): void { + // NOOP + } } registerSerializableClass(`${packageJson.version}:IDBCache`, IDBCache); diff --git a/packages/core/cache/src/LMDBCache.js b/packages/core/cache/src/LMDBCache.js index 48d5b5a30ad..0a83b0b1f56 100644 --- a/packages/core/cache/src/LMDBCache.js +++ b/packages/core/cache/src/LMDBCache.js @@ -102,6 +102,14 @@ export class LMDBCache implements Cache { async setLargeBlob(key: string, contents: Buffer | string): Promise { await this.fs.writeFile(path.join(this.dir, key), contents); } + + refresh(): void { + // Reset the read transaction for the store. This guarantees that + // the next read will see the latest changes to the store. + // Useful in scenarios where reads and writes are multi-threaded. + // See https://github.com/kriszyp/lmdb-js#resetreadtxn-void + this.store.resetReadTxn(); + } } registerSerializableClass(`${packageJson.version}:LMDBCache`, LMDBCache); diff --git a/packages/core/cache/src/types.js b/packages/core/cache/src/types.js index 1211daf844b..163b3d57644 100644 --- a/packages/core/cache/src/types.js +++ b/packages/core/cache/src/types.js @@ -14,4 +14,10 @@ export interface Cache { getLargeBlob(key: string): Promise; setLargeBlob(key: string, contents: Buffer | string): Promise; getBuffer(key: string): Promise; + /** + * In a multi-threaded environment, where there are potentially multiple Cache + * instances writing to the cache, ensure that this instance has the latest view + * of the changes that may have been written to the cache in other threads. + */ + refresh(): void; } diff --git a/packages/core/core/src/requests/WriteBundlesRequest.js b/packages/core/core/src/requests/WriteBundlesRequest.js index 499e8aa25b6..91bc06f152f 100644 --- a/packages/core/core/src/requests/WriteBundlesRequest.js +++ b/packages/core/core/src/requests/WriteBundlesRequest.js @@ -101,6 +101,18 @@ async function run({input, api, farm, options}) { let info = await api.runRequest(request); + if (!useMainThread) { + // Force a refresh of the cache to avoid a race condition + // between threaded reads and writes that can result in an LMDB cache miss: + // 1. The main thread has read some value from cache, necessitating a read transaction. + // 2. Concurrently, Thread A finishes a packaging request. + // 3. Subsequently, the main thread is tasked with this request, but fails because the read transaction is stale. + // This only occurs if the reading thread has a transaction that was created before the writing thread committed, + // and the transaction is still live when the reading thread attempts to get the written value. + // See https://github.com/parcel-bundler/parcel/issues/9121 + options.cache.refresh(); + } + bundleInfoMap[bundle.id] = info; if (!info.hashReferences.length) { hashRefToNameHash.set(