Repository navigation
Fix a deadlock condition in short term memory - #1517
Merged
Merged
Conversation
edwinyyyu
approved these changes
Aug 19, 2026
edwinyyyu
left a comment
edwinyyyu
left a comment
Contributor
There was a problem hiding this comment.
Test waits without timeouts could be expensive but that's an existing problem.
xiongzubiao
added a commit
that referenced
this pull request
Sep 30, 2026
Three defects in the ShortTermMemory lifecycle (#1719): - A cancelled query cancelled summarization for every waiter: wait_until_done() awaited the shared worker directly, so one cancelled query cancelled the worker and lost its batch. The worker is now shielded. - Clearing kept the summary of the cleared content, in memory and in the persisted copy, so after delete_session_episodes() it still reached QueryResponse and a new instance for the session restored it. clear_memory() now cancels in-flight summarization, drops the queued episodes and the summary, and overwrites the persisted copy. If a query is already waiting on that summarization, the query holds the lock, so the clear waits for it and then discards the result. The clear runs in a shielded task and holds the write lock until the cancelled worker has stopped, so a cancelled clear still finishes and no query or new batch meets the stopping worker. The episodes are cleared before the persisted copy is overwritten, so a failed save still leaves memory cleared, and the error still propagates. close() drops the in-memory summary too, but keeps the persisted copy for the next instance. - A cancelled close() left the instance open and uncleared. close() now marks the instance closed before awaiting anything and runs its cleanup in a shielded task, so a cancelled close still completes and a repeated close waits for the same cleanup. add_episodes(), clear_memory() and both query methods check for a close before taking the lock, so calls made before close() still complete, and get_summary() waits for summarization and reads the summary under one hold of the lock. close() keeps waiting for in-flight summarization rather than cancelling it: with instance_cache_size at 0, an instance is closed at the end of nearly every request, including the one whose add_episodes() started summarization, so a close() that cancelled it would cancel summarization in nearly every request that starts one. The #1517 regression test moves onto a shared gate_summarization() helper with bounded waits and a 30 s deadlock timeout, and a wrong comment in the message-length test is corrected. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_013zM4AyQyFwwUv3RNmMKwgY Signed-off-by: Zubiao Xiong <[email protected]>
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
2 participants
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.
Purpose of the change
Fix a potential deadlock in short term memory.
Description
In short term memory, if query and add episode at the same time and the memory is being summarized, the query function may get into deadlock with the add episode because of the current RWLock implementation. This PR removes the second read lock acquire to prevent deadlock.
Fixes/Closes
Fixes #1514
Type of change
[Please delete options that are not relevant.]
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
[Please delete options that are not relevant.]
Test Results: [Attach logs, screenshots, or relevant output]
Checklist
[Please delete options that are not relevant.]
Maintainer Checklist
Screenshots/Gifs
[If applicable, add screenshots or GIFs that show the changes in action. This is especially helpful for API responses. Otherwise, delete this section or type "N/A".]
Further comments
[Add any other relevant information here, such as potential side effects, future considerations, or any specific questions for the reviewer. Otherwise, type "None".]