Skip to content

Commit fe8e8bf

Browse files
andystaplesCopilot
andauthored
Harden async orchestration dispatch cleanup
Close async generators and drain or cancel pending tasks at dispatch teardown, share control-flow exception classification, and add a Windows async-runtime CI job. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 1e5bf2f commit fe8e8bf

5 files changed

Lines changed: 411 additions & 28 deletions

File tree

‎.github/workflows/durabletask.yml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,30 @@ permissions:
1414
contents: read
1515

1616
jobs:
17+
async-runtime-windows:
18+
strategy:
19+
fail-fast: false
20+
matrix:
21+
python-version: ["3.11", "3.12", "3.13", "3.14", "3.15"]
22+
runs-on: windows-latest
23+
steps:
24+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
25+
- uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0
26+
with:
27+
python-version: ${{ matrix.python-version }}
28+
- name: Install runtime test dependencies
29+
run: |
30+
python -m pip install --upgrade "pip>=25.1"
31+
python -m pip install --group test -e .
32+
- name: Check asyncio, replay, and dispatch cleanup
33+
run: >-
34+
python -m pytest
35+
tests\durabletask\test_asyncio_orchestrators.py
36+
tests\durabletask\test_coroutine_orchestrators.py
37+
tests\durabletask\test_orchestration_generator_cleanup.py
38+
tests\durabletask\test_orchestration_dispatch_recovery.py
39+
-q
40+
1741
lint-and-unit-tests:
1842
runs-on: ubuntu-latest
1943
steps:

‎durabletask/internal/asyncio_loop.py‎

Lines changed: 70 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@
99
import contextvars
1010
import math
1111
import logging
12+
import sys
1213
from collections import deque
1314
from collections.abc import Callable, Coroutine, Generator
1415
from datetime import datetime, timedelta
1516
from typing import Any, TypeVar, cast
1617

1718
from durabletask import task
19+
from durabletask.internal.exceptions import is_control_flow_exception
1820

1921
T = TypeVar("T")
2022

@@ -113,9 +115,11 @@ def __init__(
113115
self._observers: dict[int, _CompletionObserver] = {}
114116
self._completions: deque[Callable[[], None]] = deque()
115117
self._timers: dict[_DurableTimerHandle, _CompletionObserver] = {}
118+
self._asyncgens: dict[int, Any] = {}
116119
self._closed = False
117120
self._closing = False
118121
self._fatal: BaseException | None = None
122+
self._cleanup_failure: BaseException | None = None
119123
self._root: asyncio.Task[Any] | None = None
120124

121125
def time(self) -> float:
@@ -217,6 +221,8 @@ def _complete_timer(self, handle: _DurableTimerHandle) -> None:
217221
self._ready.append(handle)
218222

219223
async def wait_durable(self, durable: task.Task[T]) -> T:
224+
if self._closing:
225+
raise RuntimeError("Dispatch cleanup cannot await durable work.")
220226
if durable.is_complete:
221227
return durable.get_result()
222228
future = self.create_future()
@@ -263,7 +269,8 @@ def resume(self) -> None:
263269
raise task.TaskCancelledError(
264270
"The orchestrator coroutine was cancelled. Catch local cancellation "
265271
"inside the orchestrator; terminate the durable instance through the client.")
266-
# result() propagates the orchestrator's failure to the executor.
272+
# result() deliberately propagates control-flow BaseExceptions to the
273+
# existing executor recovery boundary, not a success-shaped fallback.
267274
result = self._root.result()
268275
raise StopIteration(result)
269276
if not self._has_durable_work():
@@ -277,33 +284,57 @@ def _has_durable_work(self) -> bool:
277284
def _drain(self) -> None:
278285
previous_loop = asyncio._get_running_loop()
279286
previous_task = asyncio.current_task(previous_loop) if previous_loop is not None else None
287+
previous_hooks = sys.get_asyncgen_hooks()
280288
if previous_task is not None and previous_loop is not None:
281289
asyncio.tasks._leave_task(previous_loop, previous_task)
282290
asyncio._set_running_loop(self)
291+
sys.set_asyncgen_hooks(firstiter=self._asyncgen_firstiter, finalizer=self._asyncgen_finalizer)
283292
try:
293+
steps = 0
294+
cleanup_limit = max(10000, 4 * (len(self._tasks) + len(self._asyncgens)))
284295
while self._ready or self._cancel_ready:
285296
self._ready.extend(handle for _, _, handle in sorted(self._cancel_ready))
286297
self._cancel_ready.clear()
287298
handle = self._ready.popleft()
288299
if not handle.cancelled():
289-
handle._run()
300+
try:
301+
handle._run()
302+
except BaseException as error:
303+
if not self._closing:
304+
raise
305+
# Defer all exits until other cleanup tasks have drained.
306+
self._record_cleanup_error(error)
290307
if self._fatal is not None:
291308
error, self._fatal = self._fatal, None
292309
if not self._closing:
293310
raise error
294-
self._logger.warning(
295-
"%s: Async cleanup callback failed.", self.ctx.instance_id, exc_info=error)
311+
self._record_cleanup_error(error)
312+
if not self._closing and self._is_complete():
313+
break
314+
if not self._closing and self._root is not None and self._root.done():
315+
break
296316
if not self._closing:
297-
if self._is_complete():
298-
break
299-
if self._root is not None and self._root.done():
300-
break
301317
self._deliver_completions()
318+
steps += 1
319+
if self._closing and steps >= cleanup_limit:
320+
self._record_cleanup_error(RuntimeError(
321+
"Async cleanup did not quiesce; cleanup must cooperate with cancellation."))
322+
self._ready.clear()
323+
self._cancel_ready.clear()
324+
break
302325
finally:
326+
sys.set_asyncgen_hooks(*previous_hooks)
303327
asyncio._set_running_loop(previous_loop)
304328
if previous_task is not None and previous_loop is not None:
305329
asyncio.tasks._enter_task(previous_loop, previous_task)
306330

331+
def _asyncgen_firstiter(self, generator: Any) -> None:
332+
self._asyncgens[id(generator)] = generator
333+
334+
def _asyncgen_finalizer(self, generator: Any) -> None:
335+
if not self._closed:
336+
self.create_task(generator.aclose())
337+
307338
def call_exception_handler(self, context: dict[str, Any]) -> None:
308339
error = context.get("exception")
309340
if isinstance(error, StopIteration):
@@ -312,26 +343,47 @@ def call_exception_handler(self, context: dict[str, Any]) -> None:
312343
error = failure
313344
self._fatal = error if isinstance(error, BaseException) else RuntimeError(context["message"])
314345

346+
def _record_cleanup_error(self, error: BaseException, source: str = "cleanup callback") -> None:
347+
self._logger.warning("%s: Async %s failed.", self.ctx.instance_id, source, exc_info=error)
348+
if not isinstance(error, (Exception, asyncio.CancelledError)):
349+
if self._cleanup_failure is None or is_control_flow_exception(error):
350+
self._cleanup_failure = error
351+
315352
def close(self) -> None:
316-
"""Cancel unfinished coroutines so their cleanup runs before the dispatch ends."""
317-
if self._closed:
318-
return
319353
self._closing = True
320354
try:
355+
completed_at_close = {future for future in self._tasks if future.done()}
356+
unobserved_at_close = {
357+
future for future in completed_at_close if getattr(future, "_log_traceback", False)}
321358
for future in tuple(self._tasks):
322359
if not future.done():
323360
future.cancel()
324361
self._drain()
325-
failure: BaseException | None = None
362+
for generator in tuple(self._asyncgens.values()):
363+
self.create_task(generator.aclose())
364+
self._drain()
365+
for future in tuple(self._tasks):
366+
if not future.done():
367+
future.cancel()
368+
self._drain()
369+
failure: BaseException | None = self._cleanup_failure
326370
for future in tuple(self._tasks):
327371
if not future.done():
328-
future.get_coro().close()
372+
try:
373+
future.get_coro().close()
374+
except BaseException as error:
375+
# Close siblings before propagating process-control exits.
376+
self._record_cleanup_error(error, "coroutine cleanup")
377+
failure = failure or error
329378
failure = failure or RuntimeError(
330379
"Async cleanup must finish without waiting for durable or external work.")
331-
elif not future.cancelled() and getattr(future, "_log_traceback", False):
332-
self._logger.warning(
333-
"%s: Async background task failed.", self.ctx.instance_id,
334-
exc_info=future.exception())
380+
elif not future.cancelled():
381+
error = future.exception()
382+
if error is not None and (future not in completed_at_close or future in unobserved_at_close):
383+
self._record_cleanup_error(
384+
error, "background task" if future in completed_at_close else "task cleanup")
385+
if self._cleanup_failure is not None:
386+
raise self._cleanup_failure
335387
if failure is not None:
336388
raise failure
337389
finally:
@@ -345,6 +397,7 @@ def close(self) -> None:
345397
self._waits.clear()
346398
self._timers.clear()
347399
self._tasks.clear()
400+
self._asyncgens.clear()
348401

349402
def run_in_executor(self, executor: Any, func: Any, *args: Any) -> asyncio.Future[Any]:
350403
raise RuntimeError("Threads/executors are unsupported in orchestrators; use an activity.")

‎durabletask/internal/exceptions.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,20 @@
11
# Copyright (c) Microsoft Corporation.
22
# Licensed under the MIT License.
33

4+
import asyncio
5+
from typing import cast
6+
47
import durabletask.internal.orchestrator_service_pb2 as pb
58

69

10+
def is_control_flow_exception(error: BaseException) -> bool:
11+
control_flow_types = (asyncio.CancelledError, GeneratorExit, KeyboardInterrupt, SystemExit)
12+
if isinstance(error, BaseExceptionGroup):
13+
group = cast(BaseExceptionGroup[BaseException], error)
14+
return group.subgroup(control_flow_types) is not None
15+
return isinstance(error, control_flow_types)
16+
17+
718
class VersionFailureException(Exception):
819
def __init__(self, error_details: pb.TaskFailureDetails) -> None:
920
super().__init__()

‎durabletask/worker.py‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,6 @@
6363
_WorkItem = tuple[Callable[..., Any], Callable[..., Any], tuple[Any, ...], dict[str, Any]]
6464

6565

66-
def _is_control_flow_exception(error: BaseException) -> bool:
67-
control_flow_types = (asyncio.CancelledError, GeneratorExit, KeyboardInterrupt, SystemExit)
68-
if isinstance(error, BaseExceptionGroup):
69-
group = cast(BaseExceptionGroup[BaseException], error)
70-
return group.subgroup(control_flow_types) is not None
71-
return isinstance(error, control_flow_types)
72-
73-
7466
class ConcurrencyOptions:
7567
"""Configuration options for controlling concurrency of different work item types and the thread pool size.
7668
@@ -1671,6 +1663,7 @@ def __init__(self,
16711663
self._exception_properties_provider = exception_properties_provider
16721664
self._logger = logger
16731665
self._async_loop: DurableEventLoop | None = None
1666+
self._async_cleanup = False
16741667

16751668
def run(self, orchestrator: Generator[task.Task[Any], Any, Any] | Coroutine[Any, Any, Any]) -> None:
16761669
if isinstance(orchestrator, Coroutine):
@@ -1683,6 +1676,7 @@ def run(self, orchestrator: Generator[task.Task[Any], Any, Any] | Coroutine[Any,
16831676

16841677
def close(self) -> None:
16851678
if self._async_loop is not None:
1679+
self._async_cleanup = True
16861680
self._async_loop.close()
16871681
elif self._generator is not None:
16881682
self._generator.close()
@@ -1849,6 +1843,8 @@ def get_actions(self) -> list[pb.OrchestratorAction]:
18491843
return current_actions
18501844

18511845
def next_sequence_number(self) -> int:
1846+
if self._async_cleanup:
1847+
raise RuntimeError("Dispatch cleanup cannot schedule durable work; use normal orchestrator control flow.")
18521848
self._sequence_number += 1
18531849
return self._sequence_number
18541850

@@ -2493,7 +2489,7 @@ def execute(
24932489
ctx.set_failed(ex)
24942490

24952491
except BaseException as ex:
2496-
if _is_control_flow_exception(ex):
2492+
if pe.is_control_flow_exception(ex):
24972493
raise
24982494
self._logger.error(
24992495
f"{instance_id}: Orchestrator {orchestration_name} exited unexpectedly.",
@@ -2527,7 +2523,7 @@ def execute(
25272523
result = ExecutionResults(
25282524
actions=actions, encoded_custom_status=ctx._encoded_custom_status, # pyright: ignore[reportPrivateUsage]
25292525
orchestration_trace_context=ctx._orchestration_trace_context, # pyright: ignore[reportPrivateUsage]
2530-
# Snapshot before generator teardown can exit a critical section.
2526+
# Snapshot before orchestrator teardown can exit a critical section.
25312527
entity_lock_releases=list(ctx._entity_context.get_lock_release_messages()), # pyright: ignore[reportPrivateUsage]
25322528
)
25332529
if unexpected_failure and result.entity_lock_releases:
@@ -2560,7 +2556,7 @@ def execute(
25602556
exc_info=True,
25612557
)
25622558
if not dispatch_exited:
2563-
if _is_control_flow_exception(ex) or result is None:
2559+
if pe.is_control_flow_exception(ex) or result is None:
25642560
raise
25652561
# Keep an existing failure rather than mask its cause.
25662562
if not any(

0 commit comments

Comments
 (0)