Skip to content

Commit 403fe48

Browse files
committed
feat(server): add aclose() to drain ActiveTask background tasks (#1101)
At shutdown the ActiveTask producer can stay pending and surface as "Task was destroyed but it is pending!". The producer's finally closes the subscriber queue with immediate=False, which joins every subscriber sink; an abandoned subscriber leaves an undrained sink so the join never returns and the producer hangs. No public API drains these background tasks today. Add aclose() to ActiveTask, ActiveTaskRegistry, and DefaultRequestHandlerV2. It force-closes the event queues (immediate=True), which releases the wedged producer, then cancels and awaits the producer and consumer tasks. ActiveTask sets _is_finished under _lock so it is mutually exclusive with start(); the registry marks itself closed so get_or_create refuses new work during teardown, closing the orphan-task race. Fixes #1101
1 parent 46b292b commit 403fe48

6 files changed

Lines changed: 313 additions & 0 deletions

File tree

src/a2a/server/agent_execution/active_task.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,43 @@ async def cancel(self, call_context: ServerCallContext) -> Task:
729729
raise RuntimeError('Task should have been created')
730730
return task
731731

732+
async def aclose(self) -> None:
733+
"""Force-closes the task's queues and drains its background tasks.
734+
735+
Provides a bounded, public teardown for the producer and consumer
736+
``asyncio.Task``s spawned in ``start()``. Without it, a producer
737+
wedged in its ``finally`` closing an abandoned subscriber sink can
738+
survive until event-loop shutdown and surface as
739+
``Task was destroyed but it is pending!``.
740+
741+
Always forces: the queues are closed with ``immediate=True`` and the
742+
background tasks are cancelled, so teardown is bounded even when a
743+
subscriber sink was never drained. It is safe to call multiple times.
744+
"""
745+
await self._event_queue_agent.close(immediate=True)
746+
await self._event_queue_subscribers.close(immediate=True)
747+
# Set `_is_finished` and collect the background tasks under `_lock` so
748+
# this is mutually exclusive with `start()`, which refuses to spawn
749+
# once `_is_finished` is set. The lock is released before awaiting the
750+
# tasks, because their teardown re-acquires it.
751+
async with self._lock:
752+
self._is_finished.set()
753+
background_tasks = [
754+
task
755+
for task in (self._producer_task, self._consumer_task)
756+
if task is not None
757+
]
758+
for task in background_tasks:
759+
task.cancel()
760+
if background_tasks:
761+
results = await asyncio.gather(
762+
*background_tasks, return_exceptions=True
763+
)
764+
for result in results:
765+
# CancelledError is a BaseException, so it is excluded here.
766+
if isinstance(result, Exception):
767+
logger.error('Error during aclose', exc_info=result)
768+
732769
async def _maybe_cleanup(self) -> None:
733770
"""Triggers cleanup if task is finished and has no subscribers.
734771

src/a2a/server/agent_execution/active_task_registry.py

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

3839
async def get_or_create(
3940
self,
@@ -44,6 +45,8 @@ async def get_or_create(
4445
) -> ActiveTask:
4546
"""Retrieves an existing ActiveTask or creates a new one."""
4647
async with self._lock:
48+
if self._closed:
49+
raise RuntimeError('ActiveTaskRegistry is closed')
4750
if task_id in self._active_tasks:
4851
return self._active_tasks[task_id]
4952

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

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: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,3 +895,90 @@ async def execute_mock(req, q):
895895
assert len(events) == 0
896896

897897
await active_task.cancel(request_context)
898+
899+
900+
@pytest.mark.timeout(5)
901+
@pytest.mark.asyncio
902+
async def test_active_task_aclose_reaps_background_tasks():
903+
"""aclose() drains a live producer and consumer."""
904+
agent_executor = Mock()
905+
task_manager = Mock()
906+
request_context = Mock(spec=RequestContext)
907+
908+
active_task = ActiveTask(
909+
agent_executor=agent_executor,
910+
task_id='test-task-id',
911+
task_manager=task_manager,
912+
push_sender=Mock(),
913+
)
914+
915+
async def slow_execute(req, q):
916+
await asyncio.sleep(10)
917+
918+
agent_executor.execute = AsyncMock(side_effect=slow_execute)
919+
task_manager.get_task = AsyncMock(
920+
return_value=Task(
921+
id='test-task-id',
922+
status=TaskStatus(state=TaskState.TASK_STATE_WORKING),
923+
)
924+
)
925+
926+
await active_task.enqueue_request(request_context)
927+
await active_task.start(
928+
call_context=ServerCallContext(), create_task_if_missing=True
929+
)
930+
931+
await active_task.aclose()
932+
933+
assert active_task._producer_task is not None
934+
assert active_task._producer_task.done()
935+
assert active_task._consumer_task is not None
936+
assert active_task._consumer_task.done()
937+
assert active_task._is_finished.is_set()
938+
939+
940+
@pytest.mark.timeout(5)
941+
@pytest.mark.asyncio
942+
async def test_active_task_aclose_force_closes_undrained_subscriber():
943+
"""aclose() unblocks past an undrained subscriber sink.
944+
945+
Reproduces issue #1101: a graceful close(immediate=False) would block
946+
forever on the leaked sink's join().
947+
"""
948+
agent_executor = Mock()
949+
task_manager = Mock()
950+
request_context = Mock(spec=RequestContext)
951+
952+
active_task = ActiveTask(
953+
agent_executor=agent_executor,
954+
task_id='test-task-id',
955+
task_manager=task_manager,
956+
push_sender=Mock(),
957+
)
958+
959+
async def slow_execute(req, q):
960+
await asyncio.sleep(10)
961+
962+
agent_executor.execute = AsyncMock(side_effect=slow_execute)
963+
task_manager.get_task = AsyncMock(
964+
return_value=Task(
965+
id='test-task-id',
966+
status=TaskStatus(state=TaskState.TASK_STATE_WORKING),
967+
)
968+
)
969+
970+
await active_task.enqueue_request(request_context)
971+
await active_task.start(
972+
call_context=ServerCallContext(), create_task_if_missing=True
973+
)
974+
975+
# Leak a subscriber sink and push an event into it without draining it.
976+
leaked = await active_task._event_queue_subscribers.tap()
977+
await active_task._event_queue_subscribers.enqueue_event(Message())
978+
await asyncio.sleep(0.05)
979+
980+
await active_task.aclose()
981+
982+
assert active_task._producer_task is not None
983+
assert active_task._producer_task.done()
984+
assert leaked.is_closed()
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

tests/server/request_handlers/test_default_request_handler_v2.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,3 +1557,37 @@ async def test_on_get_task_push_notification_config_is_owner_scoped():
15571557
),
15581558
_ctx('bob'),
15591559
)
1560+
1561+
1562+
@pytest.mark.timeout(5)
1563+
@pytest.mark.asyncio
1564+
async def test_aclose_drains_registry():
1565+
"""aclose() drains the active-task registry on shutdown."""
1566+
handler = DefaultRequestHandlerV2(
1567+
agent_executor=MockAgentExecutor(),
1568+
task_store=InMemoryTaskStore(),
1569+
agent_card=create_default_agent_card(),
1570+
)
1571+
await handler._active_task_registry.get_or_create(
1572+
'task-1',
1573+
call_context=ServerCallContext(user=UnauthenticatedUser()),
1574+
create_task_if_missing=True,
1575+
)
1576+
1577+
await handler.aclose()
1578+
1579+
assert await handler._active_task_registry.get('task-1') is None
1580+
1581+
1582+
@pytest.mark.timeout(5)
1583+
@pytest.mark.asyncio
1584+
async def test_aclose_is_idempotent_and_handles_empty():
1585+
"""aclose() is safe with no active tasks and when called twice."""
1586+
handler = DefaultRequestHandlerV2(
1587+
agent_executor=MockAgentExecutor(),
1588+
task_store=InMemoryTaskStore(),
1589+
agent_card=create_default_agent_card(),
1590+
)
1591+
1592+
await handler.aclose()
1593+
await handler.aclose()

0 commit comments

Comments
 (0)