Skip to content

Commit e0083b3

Browse files
Merge pull request #923 from YuqiGuo105:fix/memory-leak-delete-session
PiperOrigin-RevId: 892863477
2 parents 3c431e1 + fe30b2a commit e0083b3

File tree

2 files changed

+65
-7
lines changed

2 files changed

+65
-7
lines changed

core/src/main/java/com/google/adk/sessions/InMemorySessionService.java

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -186,13 +186,20 @@ public Completable deleteSession(String appName, String userId, String sessionId
186186
Objects.requireNonNull(userId, "userId cannot be null");
187187
Objects.requireNonNull(sessionId, "sessionId cannot be null");
188188

189-
ConcurrentMap<String, ConcurrentMap<String, Session>> appSessionsMap = sessions.get(appName);
190-
if (appSessionsMap != null) {
191-
ConcurrentMap<String, Session> userSessionsMap = appSessionsMap.get(userId);
192-
if (userSessionsMap != null) {
193-
userSessionsMap.remove(sessionId);
194-
}
195-
}
189+
sessions.computeIfPresent(
190+
appName,
191+
(app, appSessionsMap) -> {
192+
appSessionsMap.computeIfPresent(
193+
userId,
194+
(user, userSessionsMap) -> {
195+
userSessionsMap.remove(sessionId);
196+
// If userSessionsMap is now empty, return null to automatically remove the userId
197+
// key
198+
return userSessionsMap.isEmpty() ? null : userSessionsMap;
199+
});
200+
// If appSessionsMap is now empty, return null to automatically remove the appName key
201+
return appSessionsMap.isEmpty() ? null : appSessionsMap;
202+
});
196203
return Completable.complete();
197204
}
198205

core/src/test/java/com/google/adk/sessions/InMemorySessionServiceTest.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.adk.events.Event;
2121
import com.google.adk.events.EventActions;
2222
import io.reactivex.rxjava3.core.Single;
23+
import java.lang.reflect.Field;
2324
import java.time.Instant;
2425
import java.util.HashMap;
2526
import java.util.Optional;
@@ -266,4 +267,54 @@ public void sequentialAgents_shareTempState() {
266267
assertThat(retrievedSession.state()).doesNotContainKey("temp:agent1_output");
267268
assertThat(retrievedSession.state()).containsEntry("temp:agent2_output", "processed_data");
268269
}
270+
271+
@Test
272+
public void deleteSession_cleansUpEmptyParentMaps() throws Exception {
273+
InMemorySessionService sessionService = new InMemorySessionService();
274+
275+
Session session = sessionService.createSession("app-name", "user-id").blockingGet();
276+
277+
sessionService.deleteSession(session.appName(), session.userId(), session.id()).blockingAwait();
278+
279+
// Use reflection to access the private 'sessions' field
280+
Field field = InMemorySessionService.class.getDeclaredField("sessions");
281+
field.setAccessible(true);
282+
ConcurrentMap<?, ?> sessions = (ConcurrentMap<?, ?>) field.get(sessionService);
283+
284+
// After deleting the only session for "user-id" under "app-name",
285+
// both the userId map and the appName map should have been removed
286+
assertThat(sessions).isEmpty();
287+
}
288+
289+
@Test
290+
public void deleteSession_doesNotRemoveUserMapWhenOtherSessionsExist() throws Exception {
291+
InMemorySessionService sessionService = new InMemorySessionService();
292+
293+
Session session1 = sessionService.createSession("app-name", "user-id").blockingGet();
294+
Session session2 = sessionService.createSession("app-name", "user-id").blockingGet();
295+
296+
// Delete only one of the two sessions
297+
sessionService
298+
.deleteSession(session1.appName(), session1.userId(), session1.id())
299+
.blockingAwait();
300+
301+
// session2 should still be retrievable
302+
assertThat(
303+
sessionService
304+
.getSession(session2.appName(), session2.userId(), session2.id(), Optional.empty())
305+
.blockingGet())
306+
.isNotNull();
307+
308+
// The userId entry should still exist (not pruned) because session2 remains
309+
Field field = InMemorySessionService.class.getDeclaredField("sessions");
310+
field.setAccessible(true);
311+
@SuppressWarnings("unchecked")
312+
ConcurrentMap<String, ConcurrentMap<String, ConcurrentMap<String, ?>>> sessions =
313+
(ConcurrentMap<String, ConcurrentMap<String, ConcurrentMap<String, ?>>>)
314+
field.get(sessionService);
315+
316+
assertThat(sessions.get("app-name")).isNotNull();
317+
assertThat(sessions.get("app-name").get("user-id")).isNotNull();
318+
assertThat(sessions.get("app-name").get("user-id")).hasSize(1);
319+
}
269320
}

0 commit comments

Comments
 (0)