Skip to content

Commit c01686b

Browse files
andystaplesCopilot
andcommitted
Merge andystaples-async-stack-3-dispatch-cleanup into andystaples-async-stack-4-async-context
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2 parents 95e411c + 344196a commit c01686b

5 files changed

Lines changed: 95 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ FIXED
1313
`Any` rather than promising the child function's annotated result type.
1414
Custom result reconstruction still requires `return_type`; runtime decoding is
1515
unchanged.
16-
- Async dispatch cleanup no longer hangs when async-generator finalizers
17-
continually start successor generators; exceeding the cleanup budget is reported
18-
without changing the snapshotted response.
1916
- Activity and sub-orchestration `retry_timeout` decisions now use replay-safe
2017
orchestration time, preventing retries from expiring solely because history is
2118
replayed later. Retry calculations also accept timezone-aware timestamps.

‎docs/async-orchestrators.md‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -308,11 +308,17 @@ continue as new, encounter a history failure, or reject a version.
308308
Local cleanup can await ready work such as `sleep(0)`; it must not wait for new
309309
durable or external work.
310310
Scheduling/awaiting durable work during async teardown raises an actionable
311-
error. Teardown must cooperate with cancellation; a non-quiescent cleanup
312-
queue is bounded and reported, not allowed to run indefinitely. Each
313-
async-generator sweep has a step budget fixed at sweep entry; tasks or generators
314-
created during that sweep cannot increase it. This bounds cooperative scheduling
315-
steps, not wall-clock time or code that never yields.
311+
error. Teardown must cooperate with cancellation.
312+
313+
> [!WARNING]
314+
> Async teardown is capped at about 10,000 event-loop steps per cleanup phase
315+
> (more for dispatches with many tasks or async generators). Cleanup that is
316+
> still running when the cap is reached is truncated: remaining callbacks are
317+
> dropped, unfinished coroutines are closed, and a warning is logged. The
318+
> response decided before teardown is unchanged. Each async-generator sweep's
319+
> budget is fixed at sweep entry; tasks or generators created during that sweep
320+
> cannot increase it. The cap bounds cooperative scheduling steps, not
321+
> wall-clock time or code that never yields.
316322
317323
Ordinary teardown failures (including `CancelledError`) are logged without
318324
changing the decided response or skipping other cooperative finalizers.

‎durabletask-azuremanaged/CHANGELOG.md‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
99

1010
FIXED
1111

12-
- With the corresponding core SDK update, async dispatch cleanup no longer
13-
hangs when async-generator finalizers continually start successor generators.
14-
Exceeding the cleanup budget is reported without changing the snapshotted response.
1512
- With the corresponding core SDK update, activity and sub-orchestration
1613
`retry_timeout` decisions use replay-safe orchestration time rather than
1714
wall-clock time. See the [core upgrade guidance](../CHANGELOG.md#unreleased)

‎durabletask/internal/asyncio_loop.py‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
from typing import Any, TypeVar, cast
4040

4141
from durabletask import task
42-
from durabletask.internal.exceptions import is_control_flow_exception
4342

4443
T = TypeVar("T")
4544

@@ -169,6 +168,7 @@ def __init__(
169168
self._closing = False
170169
self._fatal: BaseException | None = None
171170
self._cleanup_failure: BaseException | None = None
171+
self._cleanup_truncated = False
172172
self._root: asyncio.Task[Any] | None = None
173173

174174
def _now(self) -> datetime:
@@ -343,7 +343,7 @@ def resume(self) -> None:
343343
def _has_durable_work(self) -> bool:
344344
return any(not durable.is_complete for durable, _ in self._waits.values()) or bool(self._timers)
345345

346-
def _drain(self) -> int:
346+
def _drain(self, cleanup_limit: int | None = None) -> int:
347347
previous_loop = asyncio._get_running_loop()
348348
previous_task = asyncio.current_task(previous_loop) if previous_loop is not None else None
349349
previous_hooks = sys.get_asyncgen_hooks()
@@ -353,7 +353,8 @@ def _drain(self) -> int:
353353
sys.set_asyncgen_hooks(firstiter=self._asyncgen_firstiter, finalizer=self._asyncgen_finalizer)
354354
try:
355355
steps = 0
356-
cleanup_limit = max(10000, 4 * (len(self._tasks) + len(self._asyncgens)))
356+
if cleanup_limit is None:
357+
cleanup_limit = self._cleanup_budget()
357358
while self._ready or self._cancel_ready:
358359
self._ready.extend(handle for _, _, handle in sorted(self._cancel_ready))
359360
self._cancel_ready.clear()
@@ -379,10 +380,7 @@ def _drain(self) -> int:
379380
self._deliver_completions()
380381
steps += 1
381382
if self._closing and steps >= cleanup_limit:
382-
self._record_cleanup_error(RuntimeError(
383-
"Async cleanup did not quiesce; cleanup must cooperate with cancellation."))
384-
self._ready.clear()
385-
self._cancel_ready.clear()
383+
self._report_unquiesced_cleanup()
386384
break
387385
return steps
388386
finally:
@@ -411,35 +409,50 @@ def call_exception_handler(self, context: dict[str, Any]) -> None:
411409
error = failure
412410
self._fatal = error if isinstance(error, BaseException) else RuntimeError(context["message"])
413411

412+
def _cleanup_budget(self) -> int:
413+
# Cooperative cleanup is capped in scheduling steps, not wall-clock time.
414+
return max(10000, 4 * (len(self._tasks) + len(self._asyncgens)))
415+
416+
def _report_unquiesced_cleanup(self) -> None:
417+
self._ready.clear()
418+
self._cancel_ready.clear()
419+
if not self._cleanup_truncated:
420+
self._cleanup_truncated = True
421+
self._record_cleanup_error(RuntimeError(
422+
"Async cleanup did not quiesce; cleanup must cooperate with cancellation."))
423+
414424
def _record_cleanup_error(self, error: BaseException, source: str = "cleanup callback") -> None:
415425
self._logger.warning("%s: Async %s failed.", self.ctx.instance_id, source, exc_info=error)
416426
if not isinstance(error, (Exception, asyncio.CancelledError)):
417427
if _is_process_control(self._cleanup_failure):
418428
return
419-
if self._cleanup_failure is None or is_control_flow_exception(error):
429+
# Keep the first failure unless a process-control exit supersedes it.
430+
if self._cleanup_failure is None or _is_process_control(error):
420431
self._cleanup_failure = error
421432

422433
def _close_asyncgens(self, closed: dict[int, Any]) -> None:
423434
# Closing one generator may first-iterate another; sweep until no new generators appear.
424435
# Closed generators stay referenced so their ids cannot be reused during cleanup.
425436
steps = 0
426437
# Cleanup-created tasks and generators must not replenish the sweep's budget.
427-
limit = max(10000, 4 * (len(self._tasks) + len(self._asyncgens)))
438+
limit = self._cleanup_budget()
428439
while True:
429440
pending = [
430441
generator for key, generator in self._asyncgens.items() if closed.get(key) is not generator]
431442
if not pending:
432443
return
433444
if steps >= limit:
434-
self._record_cleanup_error(RuntimeError(
435-
"Async cleanup did not quiesce; cleanup must cooperate with cancellation."))
445+
self._report_unquiesced_cleanup()
436446
return
437447
for generator in pending:
438448
closed[id(generator)] = generator
439449
self.create_task(generator.aclose())
440-
steps += max(1, self._drain())
450+
# Each drain spends from this sweep's remaining budget.
451+
steps += max(1, self._drain(limit - steps))
441452

442453
def close(self) -> None:
454+
if self._closed:
455+
return
443456
self._closing = True
444457
try:
445458
completed_at_close = {future for future in self._tasks if future.done()}

‎tests/durabletask/test_asyncio_orchestrators.py‎

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,6 +1126,60 @@ async def branch(index: int) -> None:
11261126
assert cleaned == [0, 1]
11271127

11281128

1129+
def test_cleanup_keeps_first_custom_exit_over_later_generator_exit() -> None:
1130+
class CleanupAbort(BaseException):
1131+
pass
1132+
1133+
errors: list[BaseException] = [CleanupAbort("first"), GeneratorExit("later")]
1134+
1135+
async def orchestrator(ctx: task.OrchestrationContext, _: Any) -> None:
1136+
async def branch(index: int) -> None:
1137+
try:
1138+
await ctx.wait_for_external_event(f"never-{index}")
1139+
finally:
1140+
raise errors[index]
1141+
1142+
asyncio.create_task(branch(0))
1143+
asyncio.create_task(branch(1))
1144+
await ctx.call_activity("wait")
1145+
1146+
complete = _completion(_execute(orchestrator, [], _start()))
1147+
assert complete.orchestrationStatus == pb.ORCHESTRATION_STATUS_FAILED
1148+
assert complete.failureDetails.errorType.endswith("CleanupAbort")
1149+
1150+
1151+
def test_dispatch_restores_host_async_generator_hooks() -> None:
1152+
def firstiter(generator: Any) -> None:
1153+
pass
1154+
1155+
def finalizer(generator: Any) -> None:
1156+
pass
1157+
1158+
async def orchestrator(ctx: task.OrchestrationContext, _: Any) -> None:
1159+
async def resource() -> AsyncIterator[int]:
1160+
yield 1
1161+
1162+
await anext(resource())
1163+
await ctx.call_activity("wait")
1164+
1165+
previous = sys.get_asyncgen_hooks()
1166+
sys.set_asyncgen_hooks(firstiter=firstiter, finalizer=finalizer)
1167+
try:
1168+
_execute(orchestrator, [], _start())
1169+
assert sys.get_asyncgen_hooks() == (firstiter, finalizer)
1170+
finally:
1171+
sys.set_asyncgen_hooks(*previous)
1172+
1173+
1174+
def test_loop_close_is_idempotent(monkeypatch: pytest.MonkeyPatch) -> None:
1175+
ctx = cast(task.OrchestrationContext, SimpleNamespace(current_utc_datetime=NOW, instance_id="id"))
1176+
loop = DurableEventLoop(ctx, is_complete=lambda: False)
1177+
loop.close()
1178+
monkeypatch.setattr(loop, "_drain", lambda *args: pytest.fail("closed loop drained again"))
1179+
loop.close()
1180+
assert loop.is_closed()
1181+
1182+
11291183
@pytest.mark.parametrize("count", [2, 32, 6000])
11301184
def test_cleanup_closes_async_generators_started_during_generator_cleanup(
11311185
count: int, caplog: pytest.LogCaptureFixture) -> None:
@@ -1146,10 +1200,13 @@ async def resource(index: int) -> AsyncIterator[int]:
11461200

11471201
actions = _execute(orchestrator, [], _start()).actions
11481202
assert [action.scheduleTask.name for action in actions] == ["wait"]
1149-
assert closed == list(range(count))
11501203
if count == 6000:
1151-
assert "Async cleanup did not quiesce" in caplog.text
1204+
# Each sweep's step budget is fixed at entry, so a long chain is truncated.
1205+
assert caplog.text.count("Async cleanup did not quiesce") == 1
1206+
assert closed[:3000] == list(range(3000))
1207+
assert len(closed) < count
11521208
else:
1209+
assert closed == list(range(count))
11531210
assert "Async" not in caplog.text
11541211

11551212

0 commit comments

Comments
 (0)