Skip to content

Commit 551bd8b

Browse files
fix(sdk): unregister atexit handler in LocalConversation.close()
LocalConversation.__init__ registers atexit.register(self.close), creating a permanent reference from the atexit handler list to the conversation object. When close() is called the registration is never removed, so the entire conversation object graph (agent, LLMs, event log, file store, all events) is pinned in memory forever. Add atexit.unregister(self.close) at the top of close() so conversations become fully GC-able after cleanup. On a long-running server processing hundreds of conversations this prevents monotonic memory growth. Closes #3136 Co-authored-by: openhands <openhands@all-hands.dev>
1 parent e529761 commit 551bd8b

2 files changed

Lines changed: 48 additions & 0 deletions

File tree

openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,9 @@ def set_security_analyzer(self, analyzer: SecurityAnalyzerBase | None) -> None:
977977

978978
def close(self) -> None:
979979
"""Close the conversation and clean up all tool executors."""
980+
# Remove the atexit reference so the conversation object can be GC'd
981+
# after close. atexit.unregister is a no-op if not registered.
982+
atexit.unregister(self.close)
980983
# Use getattr for safety - object may be partially constructed
981984
if getattr(self, "_cleanup_initiated", False):
982985
return
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
"""Tests for atexit handler cleanup to prevent memory leaks."""
2+
3+
import gc
4+
import tempfile
5+
import weakref
6+
from pathlib import Path
7+
8+
from pydantic import SecretStr
9+
10+
from openhands.sdk.agent import Agent
11+
from openhands.sdk.conversation.impl.local_conversation import LocalConversation
12+
from openhands.sdk.llm import LLM
13+
14+
15+
def _make_conversation(workspace: str) -> LocalConversation:
16+
llm = LLM(model="gpt-4o-mini", api_key=SecretStr("k"), usage_id="test")
17+
return LocalConversation(agent=Agent(llm=llm, tools=[]), workspace=workspace)
18+
19+
20+
def test_close_unregisters_atexit_handler():
21+
"""close() must remove the atexit handler so the object can be GC'd."""
22+
with tempfile.TemporaryDirectory() as tmp:
23+
workspace = str(Path(tmp) / "ws")
24+
Path(workspace).mkdir()
25+
conv = _make_conversation(workspace)
26+
27+
conv.close()
28+
29+
# If atexit still held a reference, the weak-ref would stay alive
30+
# after we drop the strong reference.
31+
ref = weakref.ref(conv)
32+
del conv
33+
gc.collect()
34+
assert ref() is None, "Conversation was not GC'd — atexit leak"
35+
36+
37+
def test_close_is_idempotent_with_atexit():
38+
"""Calling close() twice must not raise, even with atexit handling."""
39+
with tempfile.TemporaryDirectory() as tmp:
40+
workspace = str(Path(tmp) / "ws")
41+
Path(workspace).mkdir()
42+
conv = _make_conversation(workspace)
43+
44+
conv.close()
45+
conv.close() # second call is a no-op

0 commit comments

Comments
 (0)