Repository navigation
fix(Cache): release awaiters interrupted while the lookup starts - #8720
Closed
juliusmarminge wants to merge 1 commit into
Closed
juliusmarminge wants to merge 1 commit into
juliusmarminge wants to merge 1 commit into
Conversation
Cache.get forks the lookup immediately and only then returns the effect that registers the caller as an awaiter. If the lookup synchronously woke a fiber that interrupted the caller, the interrupt was delivered before that effect ran, so the awaiter was never released. The lookup was never interrupted, stayed in the cache, and every later get for the key waited on it. Register the awaiter and its release on the caller fiber before returning. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: a57c77c The changes in this PR will be included in the next version bump. This PR includes changesets to release 32 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
This branch is waiting to be deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Cache.getforks the lookup immediately (forkUnsafe(..., immediate = true)), and only then returnsentry.await(), the effect that counts the caller as an awaiter and interrupts the lookup once the last awaiter leaves. If the lookup synchronously wakes a fiber that interrupts the caller, for example by completing aDeferredthat fiber awaits, the interrupt is deferred untilCache.getreturns. It is then delivered before the returned effect starts. The awaiter is never registered or released, so:Cache.getfor that key joins the abandoned lookup and waits forever.Repro
Fix
Register the awaiter and its release on the caller fiber with
onExitUnsafebefore returning, rather than inside the returned effect. The release logic is unchanged: decrement, then interrupt the lookup if no awaiters remain and it hasn't finished. Interrupting the lookup triggers the existing observer that drops interrupted entries from the map, so the nextgetstarts a fresh lookup.The shared logic moves into
awaitEntry.Cache.getcalls it with the fiber it already has, andEntryImpl.await()wraps it inwithFiberfor the other call sites (getOption,invalidateWhen,refresh).ScopedCacheis not affected: itsawaitEntryalready installs cleanup inside an uninterruptible region beforerestore.Testing
packages/effect/test/Cache.test.ts: "interrupts the lookup when the caller is interrupted while it starts". It times out on main and passes with the fix.pnpm vitest runonCache,ScopedCache,persistence/PersistedCache,persistence/Redis,RequestResolver,Request,eventlog/EventLogRemote,eventlog/EventLogServerUnencryptedand thesql-sqlite-nodetests: 16 files, 384 tests passed.pnpm vitest run --config vitest.docs.ts packages/effect/src/Cache.ts: 28 doc examples passed.tsc -bforpackages/effect, plus a type check ofCache.test.tsagainsttsconfig.tests.json.oxlintanddprint checkon the changed files.Found while adopting Effect V4 in T3 Code.
Model/harness: Claude Opus 5.5 (1M context) via Claude Code in T3 Code.
🤖 Generated with Claude Code