Skip to content

fix(core): avoid deadlocks in cancellation callbacks - #8289

Open
林SO (Linxiushen) wants to merge 1 commit into
microsoft:mainfrom
Linxiushen:fix/cancellation-callback-reentrancy
Open

林SO (Linxiushen) wants to merge 1 commit into
microsoft:mainfrom
Linxiushen:fix/cancellation-callback-reentrancy

Conversation

@Linxiushen

Copy link
Copy Markdown

Why are these changes needed?

CancellationToken.cancel() currently invokes callbacks while holding the token's non-reentrant lock. A callback calling token.is_cancelled(), token.cancel(), or token.add_callback(...) therefore deadlocks. The same problem affects callbacks added after cancellation and cancellation propagated to a child whose callback queries its parent.

Capture the callback batch and mark the token cancelled under the lock, then invoke callbacks after releasing it. Already-cancelled registrations also invoke their callback outside the lock. Registration order within the captured batch and callback exception propagation remain unchanged; concurrent late registrations do not have a global ordering guarantee.

Regression coverage exercises both registration paths, parent/child propagation, repeated cancellation, and concurrent registration/cancellation. The unchanged implementation fails all seven reentrancy/propagation cases. The final cancellation, runtime, intervention, and tool-agent suites pass 34 tests.

Validation on Windows / Python 3.12 with the frozen autogen-core environment, using uv run --no-sync --package autogen-core:

  • python -m pytest packages/autogen-core/tests/test_cancellation.py packages/autogen-core/tests/test_runtime.py packages/autogen-core/tests/test_intervention.py packages/autogen-core/tests/test_tool_agent.py -q --tb=short — 34 passed.
  • Ruff check/format, Mypy, and Pyright on the two changed files — passed using the repository-pinned tool versions.
  • git diff --check — passed.

The full multi-package/docs/integration checks and live model calls were not run. No asyncio future/thread-safety changes are included.

Related issue number

No existing matching issue found.

Checks

  • I've included any doc changes needed for https://microsoft.github.io/autogen/. No public API or documentation changes are needed for this callback deadlock fix.
  • I've added tests (if relevant) corresponding to the changes introduced in this PR.
  • I've made sure all auto checks have passed. Local checks are listed above; hosted CI is pending.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant