gh-118950: Fix SSLProtocol.connection_lost not being called when OSError is thrown on asyncio.write. #118960
+7
−1
Conversation
This file contains 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
cjavad
requested review from
1st1,
asvetlov,
gvanrossum,
kumaraditya303 and
willingc
as code owners
|
All commit authors signed 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 |
…ocol internal transport closing state so that StreamWriter.drain will invoke sleep(0) which calls connection_lost and correctly notifies waiters of connection lost. (python#118950)
|
As requested @gvanrossum i've opened up a PR for #118950 |
cjavad
added a commit
to SimplyPrint/printer-ws-client
that referenced
this pull request
May 18, 2024
…e connection and cancellation and handle exceptions more gracefully. This addresses the 1006 issue.
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.
Let _SSLProtocolTransport.is_closing reflect SSLProtocol internal transport closing state so that StreamWriter.drain will invoke sleep(0) which calls connection_lost and correctly notifies waiters of connection lost. (#118394)
In an existing comment in
asyncio/streams.pythere is some logic that correctly ensures that theconnection_lostfunction is always called for users that keep writing and draining in some other context.The issue here is that this code only runs when the transport is marked as closing, which in the case of the _SSLTransportProtocol only happened after connection_lost has been called.
My proposed fix is for _SSLTransportProtocol.is_closing to also return true when its inner transport is marked as closing, when connection_lost is called the inner transport is removed and instead the _closed flag is set.
No additional tests have been added (yet).
asyncio.sslproto._SSLProtocolTransportcan experience invalid state, leading to silent failures. #118950