Skip to content

Commit 78641ff

Browse files
committed
fix(sdk/python): address review feedback — stop/start race + cross-loop lock (#623)
1. ResultCache.stop(): compare-before-clear so a concurrent start() that installs fresh references between the await and the cleanup doesn't get its new task orphaned. 2. AsyncExecutionManager.stop(): skip the _execution_lock section on cross-loop stop — the lock is bound to the owning loop, taking it from a foreign loop raises the same 'got Future attached to a different loop' error one line below the fix.
1 parent 1876be7 commit 78641ff

2 files changed

Lines changed: 21 additions & 9 deletions

File tree

sdk/python/agentfield/async_execution_manager.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -365,12 +365,17 @@ async def stop(self) -> None:
365365
self._event_stream_task = None
366366
self._loop = None
367367

368-
# Cancel all active executions
369-
async with self._execution_lock:
370-
for execution in self._executions.values():
371-
if execution.is_active:
372-
execution.cancel("Manager shutdown")
373-
self._release_capacity_for_execution(execution)
368+
# Cancel all active executions. The _execution_lock is an asyncio.Lock
369+
# bound to the owning loop — taking it from a foreign loop would raise
370+
# "got Future attached to a different loop". On a cross-loop stop we
371+
# skip this section: the executions will be cancelled when the owning
372+
# loop tears down or the manager is restarted (#623 review feedback).
373+
if owning_loop is current_loop:
374+
async with self._execution_lock:
375+
for execution in self._executions.values():
376+
if execution.is_active:
377+
execution.cancel("Manager shutdown")
378+
self._release_capacity_for_execution(execution)
374379

375380
# Stop components
376381
await self.connection_manager.close()

sdk/python/agentfield/result_cache.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,16 @@ async def stop(self) -> None:
236236
shutdown_event.set()
237237
await cancel_and_await_if_same_loop(task, owning_loop)
238238

239-
self._cleanup_task = None
240-
self._shutdown_event = None
241-
self._loop = None
239+
# Compare-before-clear: only null the references if they're still the
240+
# ones we snapshotted. A concurrent start() on another thread/loop
241+
# could have installed fresh references in between; clobbering those
242+
# would orphan the new cleanup task (#623 review feedback).
243+
if self._cleanup_task is task:
244+
self._cleanup_task = None
245+
if self._shutdown_event is shutdown_event:
246+
self._shutdown_event = None
247+
if self._loop is owning_loop:
248+
self._loop = None
242249

243250
with self._lock:
244251
self._cache.clear()

0 commit comments

Comments
 (0)