asyncio: avoid sharing exception object between StreamReader and close waiter (gh-156278) - #156286
Draft
aryansk wants to merge 2 commits into
Draft
asyncio: avoid sharing exception object between StreamReader and close waiter (gh-156278)#156286aryansk wants to merge 2 commits into
aryansk wants to merge 2 commits into
Conversation
…e waiter StreamReaderProtocol.connection_lost() set the same exception object on both the StreamReader's waiter and the Stream's _closed waiter. Since pythongh-90082, Future stores the traceback at set_exception() time and restores it with with_traceback() on every result() call, which mutates the exception in place. Sharing one object between two futures caused the second await (typically writer.wait_closed() in an except block) to rewrite the traceback of the in-flight exception being handled, erasing the real failure site (readexactly) and replacing it with wait_closed frames. Fix by copying the exception for the _closed waiter so each future owns an independent object. Copy falls back to reconstructing via type(exc)(*exc.args) when copy.copy fails. Fixes python#156278
|
The following commit authors need to sign the Contributor License Agreement: |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
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.
Fixes #156278
Problem
StreamReaderProtocol.connection_lost()set the same exception object on both theStreamReaderwaiter and the stream's_closedwaiter:Since gh-90082,
Futuresnapshots the traceback atset_exception()time and restores it withwith_traceback()on everyresult()call, which mutates the exception in place. Sharing one object between two futures causedawait writer.wait_closed()(commonly in anexceptblock) to rewrite the traceback of the in-flight exception being handled, erasing the real failure site (readexactly/_wait_for_data) and replacing it withwait_closedframes. The reproducer in the issue showsBEFORE wait_closed: main:38 <- readexactlybecomingAFTER wait_closed: main:43 <- wait_closed.Change
Copy the exception for the
_closedwaiter so each future owns an independent object:The reader keeps the original, the close waiter gets the copy.
with_traceback()then mutates independent objects.Validation
python -m py_compile Lib/asyncio/streams.pypassescopy.copy(exc) is not excand thatFuture.result()on one copy does not rewrite the other's__traceback__readexactly/wait_closedinterleaving) now preserves thereadexactlytraceback afterwait_closedis suppressedLib/test/test_asynciostreams tests expected to be unaffected (change is isolated toconnection_lostsharing)Notes: the interaction is between (a) sharing and (b) the
with_tracebackrestore; fixing the sharing side is the narrowest change. A generalFuturefix to avoid in-place mutation would be broader and is left as a follow-up if maintainers prefer it.