fix(core): avoid deadlocks in cancellation callbacks - #8289
Open
林SO (Linxiushen) wants to merge 1 commit into
Open
林SO (Linxiushen) wants to merge 1 commit into
林SO (Linxiushen) wants to merge 1 commit into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
CancellationToken.cancel()currently invokes callbacks while holding the token's non-reentrant lock. A callback callingtoken.is_cancelled(),token.cancel(), ortoken.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-coreenvironment, usinguv 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.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