Skip to content

Commit 7256deb

Browse files
committed
fix(toolbox-core): Attempt to close underlying managed session during gc
1 parent 270f245 commit 7256deb

File tree

2 files changed

+30
-9
lines changed

2 files changed

+30
-9
lines changed

packages/toolbox-core/src/toolbox_core/client.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# limitations under the License.
1414

1515

16+
from asyncio import get_running_loop
1617
from types import MappingProxyType
1718
from typing import Any, Awaitable, Callable, Mapping, Optional, Union
1819

@@ -140,6 +141,22 @@ async def __aexit__(self, exc_type, exc_val, exc_tb):
140141
"""
141142
await self.close()
142143

144+
def __del__(self):
145+
# This method is a "best-effort" safety net.
146+
# It should NOT be relied upon for guaranteed resource cleanup.
147+
# Explicitly using "async with" or calling "await client.close()" is the correct way.
148+
if self.__manage_session:
149+
try:
150+
loop = get_running_loop()
151+
except RuntimeError:
152+
loop = None
153+
154+
if loop and loop.is_running():
155+
# If a loop is running, try to schedule the close operation.
156+
# This is "fire-and-forget"; there's no guarantee it will complete
157+
# before the loop or interpreter shuts down.
158+
loop.create_task(self.__session.close())
159+
143160
async def close(self):
144161
"""
145162
Asynchronously closes the underlying client session. Doing so will cause
@@ -148,7 +165,7 @@ async def close(self):
148165
If the session was provided externally during initialization, the caller
149166
is responsible for its lifecycle.
150167
"""
151-
if self.__manage_session:
168+
if self.__manage_session and not self.__session.closed:
152169
await self.__session.close()
153170

154171
async def load_tool(

packages/toolbox-core/src/toolbox_core/sync_client.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
# limitations under the License.
1414

1515

16-
import asyncio
16+
from asyncio import AbstractEventLoop, new_event_loop, run_coroutine_threadsafe
1717
from threading import Thread
1818
from typing import Any, Awaitable, Callable, Mapping, Optional, Union
1919

@@ -29,7 +29,7 @@ class ToolboxSyncClient:
2929
service endpoint.
3030
"""
3131

32-
__loop: Optional[asyncio.AbstractEventLoop] = None
32+
__loop: Optional[AbstractEventLoop] = None
3333
__thread: Optional[Thread] = None
3434

3535
def __init__(
@@ -49,7 +49,7 @@ def __init__(
4949
# Running a loop in a background thread allows us to support async
5050
# methods from non-async environments.
5151
if self.__class__.__loop is None:
52-
loop = asyncio.new_event_loop()
52+
loop = new_event_loop()
5353
thread = Thread(target=loop.run_forever, daemon=True)
5454
thread.start()
5555
self.__class__.__thread = thread
@@ -58,7 +58,7 @@ def __init__(
5858
async def create_client():
5959
return ToolboxClient(url, client_headers=client_headers)
6060

61-
self.__async_client = asyncio.run_coroutine_threadsafe(
61+
self.__async_client = run_coroutine_threadsafe(
6262
create_client(), self.__class__.__loop
6363
).result()
6464

@@ -67,8 +67,12 @@ def close(self):
6767
Synchronously closes the underlying client session. Doing so will cause
6868
any tools created by this Client to cease to function.
6969
"""
70-
coro = self.__async_client.close()
71-
asyncio.run_coroutine_threadsafe(coro, self.__loop).result()
70+
if not self.__async_client.__session.closed:
71+
coro = self.__async_client.close()
72+
run_coroutine_threadsafe(coro, self.__loop).result()
73+
74+
def __del__(self):
75+
self.close()
7276

7377
def load_tool(
7478
self,
@@ -104,7 +108,7 @@ def load_tool(
104108
if not self.__loop or not self.__thread:
105109
raise ValueError("Background loop or thread cannot be None.")
106110

107-
async_tool = asyncio.run_coroutine_threadsafe(coro, self.__loop).result()
111+
async_tool = run_coroutine_threadsafe(coro, self.__loop).result()
108112
return ToolboxSyncTool(async_tool, self.__loop, self.__thread)
109113

110114
def load_toolset(
@@ -147,7 +151,7 @@ def load_toolset(
147151
if not self.__loop or not self.__thread:
148152
raise ValueError("Background loop or thread cannot be None.")
149153

150-
async_tools = asyncio.run_coroutine_threadsafe(coro, self.__loop).result()
154+
async_tools = run_coroutine_threadsafe(coro, self.__loop).result()
151155
return [
152156
ToolboxSyncTool(async_tool, self.__loop, self.__thread)
153157
for async_tool in async_tools

0 commit comments

Comments
 (0)