Fix InMemoryKache::getOrPut race condition in high concurrency#393
Open
ryansmartpadpro wants to merge 2 commits into
Open
Fix InMemoryKache::getOrPut race condition in high concurrency#393ryansmartpadpro wants to merge 2 commits into
ryansmartpadpro wants to merge 2 commits into
Conversation
The getOrPut function had a race condition where concurrent calls could result in
a NullPointerException. When multiple threads called getOrPut simultaneously on
different keys with a small cache size:
1. Thread A starts async creation and captures the deferred in creationMap
2. Thread A releases the creationMutex lock
3. The async task completes and calls creationMap.remove(key) in finally block
4. Thread A calls get(key) which looks in creationMap but finds nothing
5. Result: null is returned, causing NPE when caller expects a value
The fix captures the deferred reference while holding the lock, ensuring we can
wait for it even after it's been removed from creationMap:
val deferred = creationMutex.withLock {
if (creationMap[key] == null && map[key] == null) {
internalPutAsync(key, creationFunction)
} else {
creationMap[key]
}
}
return deferred?.let { getFromCreation(key, it) } ?: get(key)
This guarantees that getOrPut returns the successfully created value or null,
but never returns null when a value was successfully created.
Fixes MayakaApps#239
Tests that getOrPut never returns null under high concurrency when more unique keys than maxSize causes LRU evictions. Uses the real default creationScope (Dispatchers.Default) so creation deferreds run on actual threads — testInMemoryKache overrides it to the single-threaded test dispatcher which hides the race. Closes MayakaApps#239 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
Fixes #239.
getOrPuthad a race condition under high concurrency: a creation deferred could complete and remove itself fromcreationMap(via itsfinallyblock) between thecreationMutex.withLockexit and the trailingget(key)call. If the entry was also evicted by concurrent cache pressure in that window,get(key)would find nothing and returnnull.The fix captures the deferred inside the lock (before it can remove itself from
creationMap) and awaits it directly viagetFromCreation(key, deferred), returning the value from the deferred's completion state rather than re-looking it up through the map.Before:
creationMutex.withLock { if (creationMap[key] == null && map[key] == null) { @Suppress("DeferredResultUnused") internalPutAsync(key, creationFunction) } } return get(key) // deferred may have already completed and been evictedAfter:
Test plan
getOrPutHighConcurrencyregression test: launches 20,001 coroutines each callinggetOrPutwith a unique key against amaxSize = 100cache. The resulting LRU evictions reliably open the race window. The test usesInMemoryKachedirectly (nottestInMemoryKache) so creation deferreds run onDispatchers.Defaultreal threads —testInMemoryKacheoverridescreationScopeto the single-threaded test dispatcher which hides the race.jvmTestsuite passes.🤖 Generated with Claude Code