Skip to content

fix: memory leak in InMemorySessionService.deleteSession() - clean up empty parent maps#923

Open
YuqiGuo105 wants to merge 1 commit intogoogle:mainfrom
YuqiGuo105:fix/memory-leak-delete-session
Open

fix: memory leak in InMemorySessionService.deleteSession() - clean up empty parent maps#923
YuqiGuo105 wants to merge 1 commit intogoogle:mainfrom
YuqiGuo105:fix/memory-leak-delete-session

Conversation

@YuqiGuo105
Copy link

@YuqiGuo105 YuqiGuo105 commented Feb 27, 2026

Problem

Fixes #687 #687

InMemorySessionService stores sessions in a three-level nested ConcurrentHashMap:

sessions: appName  userId  sessionId  Session

deleteSession() only removed the leaf sessionId entry but never pruned the parent userId or appName keys when their child maps became empty:

BEFORE: sessions = { myApp: { user1: { ctx1: Session@123 } } }
AFTER (buggy): sessions = { myApp: { user1: {} } }    ghost empty maps!
AFTER (fixed): sessions = {}                               fully cleaned up

At scale (e.g. 1M unique users each creating and deleting one session), these empty ConcurrentHashMap objects accumulate as strongly-reachable JVM objects, causing linear memory growth and eventually OutOfMemoryError.

Fix

Replace the get() + remove() pattern with nested computeIfPresent() calls. When the remapping function returns null, ConcurrentHashMap atomically removes the key no additional locking needed.

sessions.computeIfPresent(appName, (app, appSessionsMap) -> {
    appSessionsMap.computeIfPresent(userId, (user, userSessionsMap) -> {
        userSessionsMap.remove(sessionId);
        // return null when empty  userId key is auto-removed atomically
        return userSessionsMap.isEmpty() ? null : userSessionsMap;
    });
    // return null when empty  appName key is auto-removed atomically
    return appSessionsMap.isEmpty() ? null : appSessionsMap;
});

Tests Added

Two new tests in InMemorySessionServiceTest using Java reflection to access the private sessions field:

Test Verifies
deleteSession_cleansUpEmptyParentMaps After deleting the only session, the internal sessions map is completely empty
deleteSession_doesNotRemoveUserMapWhenOtherSessionsExist When a user still has remaining sessions, the userId and appName entries are NOT prematurely pruned

All 10 InMemorySessionServiceTest tests pass (0 failures, 0 errors).

@gemini-code-assist
Copy link
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
@YuqiGuo105 YuqiGuo105 force-pushed the fix/memory-leak-delete-session branch from dd8c4d1 to 9ebdec6 Compare February 27, 2026 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deleteSession() of the InMemorySessionService does not delete the user to session mapping

1 participant