fix: memory leak in InMemorySessionService.deleteSession() - clean up empty parent maps#923
Open
YuqiGuo105 wants to merge 1 commit intogoogle:mainfrom
Open
fix: memory leak in InMemorySessionService.deleteSession() - clean up empty parent maps#923YuqiGuo105 wants to merge 1 commit intogoogle:mainfrom
YuqiGuo105 wants to merge 1 commit intogoogle:mainfrom
Conversation
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
… empty parent maps
When deleteSession() removes the last session for a userId, the now-empty
userId map remained in the parent appName map. Similarly, if all users under
an appName were deleted, the empty appName map remained in the top-level
sessions map. This caused linear memory growth with the number of unique
users/apps, eventually leading to OutOfMemoryError.
Replace the manual get+remove pattern with computeIfPresent() calls that
atomically remove parent map entries when they become empty. When the
remapping function returns null, ConcurrentHashMap automatically removes
the key - this is thread-safe without additional locking.
Before:
sessions = { appName: { userId: {} } } <-- empty maps leak
After:
sessions = {} <-- fully cleaned up
Add two new tests:
- deleteSession_cleansUpEmptyParentMaps: verifies via reflection that the
internal sessions map is completely empty after deleting the only session
- deleteSession_doesNotRemoveUserMapWhenOtherSessionsExist: verifies that
a userId entry is NOT pruned when the user still has remaining sessions
Fixes google#687
dd8c4d1 to
9ebdec6
Compare
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
Fixes #687 #687
InMemorySessionServicestores sessions in a three-level nestedConcurrentHashMap:deleteSession()only removed the leafsessionIdentry but never pruned the parentuserIdorappNamekeys when their child maps became empty:At scale (e.g. 1M unique users each creating and deleting one session), these empty
ConcurrentHashMapobjects accumulate as strongly-reachable JVM objects, causing linear memory growth and eventuallyOutOfMemoryError.Fix
Replace the
get()+remove()pattern with nestedcomputeIfPresent()calls. When the remapping function returnsnull,ConcurrentHashMapatomically removes the key no additional locking needed.Tests Added
Two new tests in
InMemorySessionServiceTestusing Java reflection to access the privatesessionsfield:deleteSession_cleansUpEmptyParentMapssessionsmap is completely emptydeleteSession_doesNotRemoveUserMapWhenOtherSessionsExistuserIdandappNameentries are NOT prematurely prunedAll 10
InMemorySessionServiceTesttests pass (0 failures, 0 errors).