Skip to content

Commit 9801f46

Browse files
authored
feat(server): add aclose() to drain ActiveTask background tasks (#1101) (#1105)
## Summary Adds a public `aclose()` teardown to `ActiveTask`, `ActiveTaskRegistry`, and `DefaultRequestHandlerV2` that force-drains the SDK-owned producer, consumer, and dispatcher `asyncio.Task`s so none are left pending at event-loop shutdown. - `ActiveTask.aclose()` force-closes both event queues and cancels the producer and consumer tasks, then awaits them. It sets `_is_finished` under `_lock`, so it is mutually exclusive with `start()` (which refuses to spawn once `_is_finished` is set). - `ActiveTaskRegistry.aclose()` marks the registry closed so `get_or_create` refuses new work, drains every active task, then awaits the in-flight `_remove_task` cleanup tasks. The lock is released before awaiting because `_remove_task` re-acquires it. - `DefaultRequestHandlerV2.aclose()` delegates to the registry drain, for wiring into an ASGI `lifespan` / `on_shutdown` hook. ## Why Fixes #1101. At shutdown the `ActiveTask` producer can stay pending and surface as `Task was destroyed but it is pending!`. The producer's `finally` calls `_event_queue_subscribers.close(immediate=False)`, which awaits `join()` on every subscriber sink; an abandoned subscriber leaves an undrained sink, so the `join()` never returns and the producer hangs. There is no public way to drain these background tasks today. `aclose()` closes the queues with `immediate=True`, which releases the wedged producer, and reaps the tasks. The teardown always forces rather than exposing a graceful `immediate=False` option, because that path inherits the documented `close(immediate=False)` deadlock and a shutdown hook must be bounded. ## Test plan - [x] `uv run pytest tests/server/agent_execution/ tests/server/request_handlers/ tests/server/events/` — pass - [x] `uv run pytest --cov=a2a --cov-fail-under=88` — pass - [x] `./scripts/lint.sh` — ruff, ruff-format, and ty clean New tests cover the registry drain, idempotency, empty registry, new-work rejected after close, and an errored task being logged rather than propagated; `ActiveTask` reaping a running producer and force-closing past an undrained subscriber (the #1101 repro); and the handler drain. Fixes #1101 🦕
1 parent d19c4d2 commit 9801f46

8 files changed

Lines changed: 417 additions & 0 deletions

File tree

src/a2a/server/agent_execution/active_task.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,48 @@ async def cancel(self, call_context: ServerCallContext) -> Task:
755755
raise RuntimeError('Task should have been created')
756756
return task
757757

758+
async def aclose(self) -> None:
759+
"""Force-closes the task's queues and drains its background tasks.
760+
761+
Provides a bounded, public teardown for the producer and consumer
762+
``asyncio.Task``s spawned in ``start()``. Without it, a producer
763+
wedged in its ``finally`` closing an abandoned subscriber sink can
764+
survive until event-loop shutdown and surface as
765+
``Task was destroyed but it is pending!``.
766+
767+
Always forces: the queues are closed with ``immediate=True`` and the
768+
background tasks are cancelled, so teardown is bounded even when a
769+
subscriber sink was never drained. It is safe to call multiple times.
770+
"""
771+
# Shut down the request queue first, mirroring the producer's
772+
# ``finally``. If `start()` was never called, the producer/consumer
773+
# ``finally`` blocks never run, and without this a caller parked in
774+
# `enqueue_request()` would wait forever.
775+
self._request_queue.shutdown(immediate=True)
776+
await self._event_queue_agent.close(immediate=True)
777+
await self._event_queue_subscribers.close(immediate=True)
778+
# Set `_is_finished` and collect the background tasks under `_lock` so
779+
# this is mutually exclusive with `start()`, which refuses to spawn
780+
# once `_is_finished` is set. The lock is released before awaiting the
781+
# tasks, because their teardown re-acquires it.
782+
async with self._lock:
783+
self._is_finished.set()
784+
background_tasks = [
785+
task
786+
for task in (self._producer_task, self._consumer_task)
787+
if task is not None
788+
]
789+
for task in background_tasks:
790+
task.cancel()
791+
if background_tasks:
792+
results = await asyncio.gather(
793+
*background_tasks, return_exceptions=True
794+
)
795+
for result in results:
796+
# CancelledError is a BaseException, so it is excluded here.
797+
if isinstance(result, Exception):
798+
logger.error('Error during aclose', exc_info=result)
799+
758800
async def _maybe_cleanup(self) -> None:
759801
"""Triggers cleanup if task is finished and has no subscribers.
760802

src/a2a/server/agent_execution/active_task_registry.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ def __init__(
3535
self._active_tasks: dict[str, ActiveTask] = {}
3636
self._lock = asyncio.Lock()
3737
self._cleanup_tasks: set[asyncio.Task[None]] = set()
38+
self._closed = False
3839

3940
async def get_or_create(
4041
self,
@@ -46,6 +47,8 @@ async def get_or_create(
4647
) -> ActiveTask:
4748
"""Retrieves an existing ActiveTask or creates a new one."""
4849
async with self._lock:
50+
if self._closed:
51+
raise RuntimeError('ActiveTaskRegistry is closed')
4952
if task_id in self._active_tasks:
5053
return self._active_tasks[task_id]
5154

@@ -88,3 +91,39 @@ async def get(self, task_id: str) -> ActiveTask | None:
8891
"""Retrieves an existing task."""
8992
async with self._lock:
9093
return self._active_tasks.get(task_id)
94+
95+
async def aclose(self) -> None:
96+
"""Closes the registry and drains all active tasks.
97+
98+
Marks the registry closed so ``get_or_create`` refuses new work, then
99+
force-closes every registered ``ActiveTask`` and awaits the in-flight
100+
``_remove_task`` cleanup tasks they schedule, so no SDK-owned
101+
``asyncio.Task`` is left pending at event-loop shutdown. Safe to call
102+
multiple times.
103+
104+
The close flag is set and the active-task snapshot is taken under
105+
``_lock``, and the lock is then released before awaiting, because
106+
``_remove_task`` re-acquires ``_lock``; holding it while draining
107+
would deadlock. Marking closed under the same lock prevents a
108+
concurrent ``get_or_create`` from registering a task that the drain
109+
would miss.
110+
"""
111+
async with self._lock:
112+
self._closed = True
113+
active_tasks = list(self._active_tasks.values())
114+
115+
if active_tasks:
116+
results = await asyncio.gather(
117+
*(task.aclose() for task in active_tasks),
118+
return_exceptions=True,
119+
)
120+
for result in results:
121+
if isinstance(result, Exception):
122+
logger.error('Error draining active task', exc_info=result)
123+
124+
cleanup_tasks = list(self._cleanup_tasks)
125+
if cleanup_tasks:
126+
await asyncio.gather(*cleanup_tasks, return_exceptions=True)
127+
128+
async with self._lock:
129+
self._active_tasks.clear()

src/a2a/server/events/event_queue_v2.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,21 @@ async def close(self, immediate: bool = False) -> None:
239239
*(sink.close(immediate=False) for sink in sinks_to_close)
240240
)
241241

242+
# Both branches above cancel the dispatcher task, but the sink gather
243+
# only awaits the sinks. Await the cancelled dispatcher here so it is
244+
# never left pending when close() returns -- e.g. a source created with
245+
# create_default_sink=False and no taps has no sinks for the gather to
246+
# await, which would otherwise leave the cancelled dispatcher pending
247+
# and trigger "Task was destroyed but it is pending!".
248+
#
249+
# Suppress the expected CancelledError and any error the dispatcher
250+
# raised while shutting down: close() is teardown (reachable from
251+
# __aexit__) and must not raise, and _dispatch_loop already logs its own
252+
# exceptions, so a dispatcher failure stays observable without
253+
# propagating out of close().
254+
with contextlib.suppress(asyncio.CancelledError, Exception):
255+
await self._dispatcher_task
256+
242257
def is_closed(self) -> bool:
243258
"""[DEPRECATED] Checks if the queue is closed.
244259

src/a2a/server/request_handlers/default_request_handler_v2.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,15 @@ def __init__( # noqa: PLR0913
112112
)
113113
self._background_tasks = set()
114114

115+
async def aclose(self) -> None:
116+
"""Shuts down the handler, draining all active tasks.
117+
118+
Drains the ``ActiveTaskRegistry`` so a server shutdown leaves no
119+
pending ``asyncio.Task``. Intended to be wired into an ASGI
120+
``lifespan`` / ``on_shutdown`` hook. Safe to call multiple times.
121+
"""
122+
await self._active_task_registry.aclose()
123+
115124
@validate_request_params
116125
async def on_get_task( # noqa: D102
117126
self,

tests/server/agent_execution/test_active_task.py

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
TaskStatus,
2121
TaskStatusUpdateEvent,
2222
)
23+
from a2a.utils._async_queue_compat import QueueShutDown, create_async_queue
2324
from a2a.utils.errors import InvalidParamsError
2425

2526

@@ -979,3 +980,125 @@ async def execute_mock(req, q):
979980
assert cleanup_called, (
980981
'on_cleanup should be called after producer is drained'
981982
)
983+
984+
985+
@pytest.mark.timeout(5)
986+
@pytest.mark.asyncio
987+
async def test_active_task_aclose_reaps_background_tasks():
988+
"""aclose() drains a live producer and consumer."""
989+
agent_executor = Mock()
990+
task_manager = Mock()
991+
request_context = Mock(spec=RequestContext)
992+
993+
active_task = ActiveTask(
994+
agent_executor=agent_executor,
995+
task_id='test-task-id',
996+
task_manager=task_manager,
997+
push_sender=Mock(),
998+
)
999+
1000+
async def slow_execute(req, q):
1001+
await asyncio.sleep(10)
1002+
1003+
agent_executor.execute = AsyncMock(side_effect=slow_execute)
1004+
task_manager.get_task = AsyncMock(
1005+
return_value=Task(
1006+
id='test-task-id',
1007+
status=TaskStatus(state=TaskState.TASK_STATE_WORKING),
1008+
)
1009+
)
1010+
1011+
await active_task.enqueue_request(request_context)
1012+
await active_task.start(
1013+
call_context=ServerCallContext(), create_task_if_missing=True
1014+
)
1015+
1016+
await active_task.aclose()
1017+
1018+
assert active_task._producer_task is not None
1019+
assert active_task._producer_task.done()
1020+
assert active_task._consumer_task is not None
1021+
assert active_task._consumer_task.done()
1022+
assert active_task._is_finished.is_set()
1023+
1024+
1025+
@pytest.mark.timeout(5)
1026+
@pytest.mark.asyncio
1027+
async def test_active_task_aclose_force_closes_undrained_subscriber():
1028+
"""aclose() unblocks past an undrained subscriber sink.
1029+
1030+
Reproduces issue #1101: a graceful close(immediate=False) would block
1031+
forever on the leaked sink's join().
1032+
"""
1033+
agent_executor = Mock()
1034+
task_manager = Mock()
1035+
request_context = Mock(spec=RequestContext)
1036+
1037+
active_task = ActiveTask(
1038+
agent_executor=agent_executor,
1039+
task_id='test-task-id',
1040+
task_manager=task_manager,
1041+
push_sender=Mock(),
1042+
)
1043+
1044+
async def slow_execute(req, q):
1045+
await asyncio.sleep(10)
1046+
1047+
agent_executor.execute = AsyncMock(side_effect=slow_execute)
1048+
task_manager.get_task = AsyncMock(
1049+
return_value=Task(
1050+
id='test-task-id',
1051+
status=TaskStatus(state=TaskState.TASK_STATE_WORKING),
1052+
)
1053+
)
1054+
1055+
await active_task.enqueue_request(request_context)
1056+
await active_task.start(
1057+
call_context=ServerCallContext(), create_task_if_missing=True
1058+
)
1059+
1060+
# Leak a subscriber sink and push an event into it without draining it.
1061+
leaked = await active_task._event_queue_subscribers.tap()
1062+
await active_task._event_queue_subscribers.enqueue_event(Message())
1063+
await asyncio.sleep(0.05)
1064+
1065+
await active_task.aclose()
1066+
1067+
assert active_task._producer_task is not None
1068+
assert active_task._producer_task.done()
1069+
assert leaked.is_closed()
1070+
1071+
1072+
@pytest.mark.timeout(5)
1073+
@pytest.mark.asyncio
1074+
async def test_active_task_aclose_never_started_shuts_down_request_queue():
1075+
"""aclose() on a never-started task shuts down the request queue.
1076+
1077+
If start() was never called, the producer and consumer `finally` blocks
1078+
that normally shut down `_request_queue` never run, so without this a
1079+
caller parked in `enqueue_request()` would never be released.
1080+
"""
1081+
active_task = ActiveTask(
1082+
agent_executor=Mock(),
1083+
task_id='test-task-id',
1084+
task_manager=Mock(),
1085+
push_sender=Mock(),
1086+
)
1087+
1088+
# Swap in a bounded queue and fill it so the next enqueue_request parks.
1089+
active_task._request_queue = create_async_queue(maxsize=1)
1090+
await active_task.enqueue_request(Mock(spec=RequestContext))
1091+
waiter = asyncio.create_task(
1092+
active_task.enqueue_request(Mock(spec=RequestContext))
1093+
)
1094+
await asyncio.sleep(0.05)
1095+
assert not waiter.done()
1096+
1097+
await active_task.aclose()
1098+
1099+
# The parked waiter is released with the shutdown error, and any
1100+
# subsequent enqueue fails fast instead of hanging.
1101+
with pytest.raises(QueueShutDown):
1102+
await waiter
1103+
with pytest.raises(QueueShutDown):
1104+
await active_task.enqueue_request(Mock(spec=RequestContext))
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import asyncio
2+
import logging
3+
4+
from unittest.mock import AsyncMock
5+
6+
import pytest
7+
8+
from a2a.server.agent_execution.active_task_registry import ActiveTaskRegistry
9+
from a2a.server.agent_execution.agent_executor import AgentExecutor
10+
from a2a.server.agent_execution.context import RequestContext
11+
from a2a.server.context import ServerCallContext
12+
from a2a.server.events.event_queue_v2 import EventQueue
13+
from a2a.server.tasks import InMemoryTaskStore
14+
15+
16+
class _SlowExecutor(AgentExecutor):
17+
"""An executor whose execute() blocks until cancelled."""
18+
19+
async def execute(
20+
self, context: RequestContext, event_queue: EventQueue
21+
) -> None:
22+
await asyncio.sleep(10)
23+
24+
async def cancel(
25+
self, context: RequestContext, event_queue: EventQueue
26+
) -> None:
27+
return None
28+
29+
30+
def _make_registry() -> ActiveTaskRegistry:
31+
return ActiveTaskRegistry(
32+
agent_executor=_SlowExecutor(),
33+
task_store=InMemoryTaskStore(),
34+
)
35+
36+
37+
@pytest.mark.timeout(5)
38+
@pytest.mark.asyncio
39+
async def test_aclose_reaps_active_tasks_and_empties_registry():
40+
"""aclose() reaps background tasks and removes them."""
41+
registry = _make_registry()
42+
active = await registry.get_or_create(
43+
'task-1',
44+
call_context=ServerCallContext(),
45+
create_task_if_missing=True,
46+
)
47+
48+
await registry.aclose()
49+
50+
assert active._producer_task is not None
51+
assert active._producer_task.done()
52+
assert active._consumer_task is not None
53+
assert active._consumer_task.done()
54+
assert await registry.get('task-1') is None
55+
56+
57+
@pytest.mark.timeout(5)
58+
@pytest.mark.asyncio
59+
async def test_aclose_is_idempotent():
60+
"""Calling aclose() repeatedly is a safe no-op."""
61+
registry = _make_registry()
62+
await registry.get_or_create(
63+
'task-1',
64+
call_context=ServerCallContext(),
65+
create_task_if_missing=True,
66+
)
67+
68+
await registry.aclose()
69+
await registry.aclose()
70+
71+
72+
@pytest.mark.timeout(5)
73+
@pytest.mark.asyncio
74+
async def test_aclose_on_empty_registry():
75+
"""aclose() with no active tasks returns immediately."""
76+
registry = _make_registry()
77+
await registry.aclose()
78+
79+
80+
@pytest.mark.timeout(5)
81+
@pytest.mark.asyncio
82+
async def test_get_or_create_rejected_after_aclose():
83+
"""A closed registry refuses to create new tasks (no orphan race)."""
84+
registry = _make_registry()
85+
await registry.aclose()
86+
87+
with pytest.raises(RuntimeError):
88+
await registry.get_or_create(
89+
'task-1',
90+
call_context=ServerCallContext(),
91+
create_task_if_missing=True,
92+
)
93+
94+
95+
@pytest.mark.timeout(5)
96+
@pytest.mark.asyncio
97+
async def test_aclose_logs_and_swallows_task_errors(caplog):
98+
"""A failing ActiveTask.aclose is logged, not propagated."""
99+
registry = _make_registry()
100+
failing = AsyncMock()
101+
failing.aclose = AsyncMock(side_effect=ValueError('boom'))
102+
registry._active_tasks['bad'] = failing
103+
104+
with caplog.at_level(logging.ERROR):
105+
await registry.aclose()
106+
107+
assert 'Error draining active task' in caplog.text

0 commit comments

Comments
 (0)