Skip to content

Commit 0a1d469

Browse files
andystaplesCopilot
andcommitted
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 0cfb7b3 commit 0a1d469

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,13 +9,15 @@
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 fractions import Fraction
1617
from typing import Any, TypeVar, cast
1718

1819
from durabletask import task
20+
from durabletask.internal.exceptions import is_control_flow_exception
1921

2022
T = TypeVar("T")
2123

@@ -130,9 +132,11 @@ def __init__(
130132
self._observers: dict[int, _CompletionObserver] = {}
131133
self._completions: deque[Callable[[], None]] = deque()
132134
self._timers: dict[_DurableTimerHandle, _CompletionObserver] = {}
135+
self._asyncgens: dict[int, Any] = {}
133136
self._closed = False
134137
self._closing = False
135138
self._fatal: BaseException | None = None
139+
self._cleanup_failure: BaseException | None = None
136140
self._root: asyncio.Task[Any] | None = None
137141

138142
def time(self) -> float:
@@ -237,6 +241,8 @@ def _complete_timer(self, handle: _DurableTimerHandle) -> None:
237241
self._ready.append(handle)
238242

239243
async def wait_durable(self, durable: task.Task[T]) -> T:
244+
if self._closing:
245+
raise RuntimeError("Dispatch cleanup cannot await durable work.")
240246
if durable.is_complete:
241247
return durable.get_result()
242248
future = self.create_future()
@@ -283,7 +289,8 @@ def resume(self) -> None:
283289
raise task.TaskCancelledError(
284290
"The orchestrator coroutine was cancelled. Catch local cancellation "
285291
"inside the orchestrator; terminate the durable instance through the client.")
286-
# result() propagates the orchestrator's failure to the executor.
292+
# result() deliberately propagates control-flow BaseExceptions to the
293+
# existing executor recovery boundary, not a success-shaped fallback.
287294
result = self._root.result()
288295
raise StopIteration(result)
289296
if not self._has_durable_work():
@@ -297,33 +304,57 @@ def _has_durable_work(self) -> bool:
297304
def _drain(self) -> None:
298305
previous_loop = asyncio._get_running_loop()
299306
previous_task = asyncio.current_task(previous_loop) if previous_loop is not None else None
307+
previous_hooks = sys.get_asyncgen_hooks()
300308
if previous_task is not None and previous_loop is not None:
301309
asyncio.tasks._leave_task(previous_loop, previous_task)
302310
asyncio._set_running_loop(self)
311+
sys.set_asyncgen_hooks(firstiter=self._asyncgen_firstiter, finalizer=self._asyncgen_finalizer)
303312
try:
313+
steps = 0
314+
cleanup_limit = max(10000, 4 * (len(self._tasks) + len(self._asyncgens)))
304315
while self._ready or self._cancel_ready:
305316
self._ready.extend(handle for _, _, handle in sorted(self._cancel_ready))
306317
self._cancel_ready.clear()
307318
handle = self._ready.popleft()
308319
if not handle.cancelled():
309-
handle._run()
320+
try:
321+
handle._run()
322+
except BaseException as error:
323+
if not self._closing:
324+
raise
325+
# Defer all exits until other cleanup tasks have drained.
326+
self._record_cleanup_error(error)
310327
if self._fatal is not None:
311328
error, self._fatal = self._fatal, None
312329
if not self._closing:
313330
raise error
314-
self._logger.warning(
315-
"%s: Async cleanup callback failed.", self.ctx.instance_id, exc_info=error)
331+
self._record_cleanup_error(error)
332+
if not self._closing and self._is_complete():
333+
break
334+
if not self._closing and self._root is not None and self._root.done():
335+
break
316336
if not self._closing:
317-
if self._is_complete():
318-
break
319-
if self._root is not None and self._root.done():
320-
break
321337
self._deliver_completions()
338+
steps += 1
339+
if self._closing and steps >= cleanup_limit:
340+
self._record_cleanup_error(RuntimeError(
341+
"Async cleanup did not quiesce; cleanup must cooperate with cancellation."))
342+
self._ready.clear()
343+
self._cancel_ready.clear()
344+
break
322345
finally:
346+
sys.set_asyncgen_hooks(*previous_hooks)
323347
asyncio._set_running_loop(previous_loop)
324348
if previous_task is not None and previous_loop is not None:
325349
asyncio.tasks._enter_task(previous_loop, previous_task)
326350

351+
def _asyncgen_firstiter(self, generator: Any) -> None:
352+
self._asyncgens[id(generator)] = generator
353+
354+
def _asyncgen_finalizer(self, generator: Any) -> None:
355+
if not self._closed:
356+
self.create_task(generator.aclose())
357+
327358
def call_exception_handler(self, context: dict[str, Any]) -> None:
328359
error = context.get("exception")
329360
if isinstance(error, StopIteration):
@@ -332,26 +363,47 @@ def call_exception_handler(self, context: dict[str, Any]) -> None:
332363
error = failure
333364
self._fatal = error if isinstance(error, BaseException) else RuntimeError(context["message"])
334365

366+
def _record_cleanup_error(self, error: BaseException, source: str = "cleanup callback") -> None:
367+
self._logger.warning("%s: Async %s failed.", self.ctx.instance_id, source, exc_info=error)
368+
if not isinstance(error, (Exception, asyncio.CancelledError)):
369+
if self._cleanup_failure is None or is_control_flow_exception(error):
370+
self._cleanup_failure = error
371+
335372
def close(self) -> None:
336-
"""Cancel unfinished coroutines so their cleanup runs before the dispatch ends."""
337-
if self._closed:
338-
return
339373
self._closing = True
340374
try:
375+
completed_at_close = {future for future in self._tasks if future.done()}
376+
unobserved_at_close = {
377+
future for future in completed_at_close if getattr(future, "_log_traceback", False)}
341378
for future in tuple(self._tasks):
342379
if not future.done():
343380
future.cancel()
344381
self._drain()
345-
failure: BaseException | None = None
382+
for generator in tuple(self._asyncgens.values()):
383+
self.create_task(generator.aclose())
384+
self._drain()
385+
for future in tuple(self._tasks):
386+
if not future.done():
387+
future.cancel()
388+
self._drain()
389+
failure: BaseException | None = self._cleanup_failure
346390
for future in tuple(self._tasks):
347391
if not future.done():
348-
future.get_coro().close()
392+
try:
393+
future.get_coro().close()
394+
except BaseException as error:
395+
# Close siblings before propagating process-control exits.
396+
self._record_cleanup_error(error, "coroutine cleanup")
397+
failure = failure or error
349398
failure = failure or RuntimeError(
350399
"Async cleanup must finish without waiting for durable or external work.")
351-
elif not future.cancelled() and getattr(future, "_log_traceback", False):
352-
self._logger.warning(
353-
"%s: Async background task failed.", self.ctx.instance_id,
354-
exc_info=future.exception())
400+
elif not future.cancelled():
401+
error = future.exception()
402+
if error is not None and (future not in completed_at_close or future in unobserved_at_close):
403+
self._record_cleanup_error(
404+
error, "background task" if future in completed_at_close else "task cleanup")
405+
if self._cleanup_failure is not None:
406+
raise self._cleanup_failure
355407
if failure is not None:
356408
raise failure
357409
finally:
@@ -365,6 +417,7 @@ def close(self) -> None:
365417
self._waits.clear()
366418
self._timers.clear()
367419
self._tasks.clear()
420+
self._asyncgens.clear()
368421

369422
def run_in_executor(self, executor: Any, func: Any, *args: Any) -> asyncio.Future[Any]:
370423
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)