Fix Cancelled.__context__ leaking from other tasks#3395
Closed
Fridayai700 wants to merge 1 commit intopython-trio:mainfrom
Closed
Fix Cancelled.__context__ leaking from other tasks#3395Fridayai700 wants to merge 1 commit intopython-trio:mainfrom
Fridayai700 wants to merge 1 commit intopython-trio:mainfrom
Conversation
When task 1 calls cancel() while handling an exception, the Cancelled exception delivered to task 2 incorrectly inherits task 1's exception as __context__. This happens because capture(raise_cancel) is called within task 1's exception handling context, so Python automatically sets the __context__ on the newly-raised Cancelled. Fix by clearing __context__ on the captured error before rescheduling. This matches the behavior of other abort pathways (deferred cancellation like IOCP/io_uring) where the Cancelled exception gets __context__ = None. Fixes python-trio#2649
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3395 +/- ##
===============================================
Coverage 100.00000% 100.00000%
===============================================
Files 128 128
Lines 19424 19447 +23
Branches 1318 1318
===============================================
+ Hits 19424 19447 +23
🚀 New features to boost your workflow:
|
Contributor
|
We have a cleaner way to do this, blocked on some other PRs. |
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.
Summary
Fixes #2649
When task 1 calls
cancel()on a scope while handling an exception, theCancelledexception delivered to task 2 (sleeping in that scope) incorrectly inherits task 1's exception as__context__. This is becausecapture(raise_cancel)is called within task 1's exception handling context, so Python automatically sets__context__on the newly-raisedCancelled.Before
After
# task2's Cancelled.__context__ is None ← correctFix
Clear
__context__on the captured error in_attempt_abortbefore rescheduling the cancelled task. This matches the behavior of other abort pathways (deferred cancellation like IOCP/io_uring) where theCancelledexception naturally gets__context__ = None.The local variable is explicitly deleted to avoid creating cyclic garbage (verified by the existing
test_simple_cancel_scope_usage_doesnt_create_cyclic_garbagetest).Test
Added
test_cancelled_context_does_not_leak_from_other_taskwhich:__context__ is NoneThe test fails without the fix (Cancelled.context == RuntimeError) and passes with it.