Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

bpo-36801: Fix waiting in StreamWriter.drain for closing SSL transport#13098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

asvetlov
Copy link
Contributor

@asvetlovasvetlov commentedMay 5, 2019•
edited by bedevere-bot
Loading

@asvetlov
Copy link
ContributorAuthor

The backport to 3.7 is desired.
The problem not in SSL connections only.
Working on the patch I've found thatawait subproc.stdin.wait_closed() is broken on 3.7
Probably nobody executes this call though, nobody blames.

# ConnectionResetError otherwise
fut = self._protocol._get_close_waiter(self)
await fut
raise ConnectionResetError('Connection lost')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Do we want to raise an error from drain()? Maybe it should just return? What's the point of this error?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

That's howprotocol._drain_helper() works now.
The future is set to exception only if the connection is closed with a failure.
Butawait writer.drain() raisesConnectionResetError.
I believe this is good:await writer.write(b'data') should fail loudly if the socket is closed.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Maybe the code could be simplified by setting ConnectionResetError byconnection_lost() callback handler but I'd like to keep the PR small.
Future improvement worth another pull request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

OK, got it.

@miss-islington
Copy link
Contributor

Thanks@asvetlov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 7, 2019
@bedevere-bot
Copy link

GH-13176 is a backport of this pull request to the3.7 branch.

miss-islington added a commit that referenced this pull requestMay 7, 2019
GH-13098)https://bugs.python.org/issue36801(cherry picked from commit1cc0ee7)Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
vstinner added a commit that referenced this pull requestMay 14, 2019
@asvetlovasvetlov deleted the streams-wait-ssl-shutdown branchSeptember 12, 2019 12:43
@nullstd
Copy link

Hi@asvetlov , I ran into aBrokenPipeError error when testing the following code:

#!/usr/bin/env pythonimport sysimport osimport asyncioasync def main_task():    print(sys.version)    xxx = "x" * 196428    incoming_original_data = xxx.encode()    print(f"stdin len: {len(incoming_original_data)}")    proc = await asyncio.create_subprocess_shell(        f"sleep 2 && xxd -l 1",        stdin=asyncio.subprocess.PIPE,        stdout=asyncio.subprocess.PIPE,        stderr=asyncio.subprocess.PIPE)        await proc.communicate(input = incoming_original_data)    print("subprocess ended")def main():    opts = asyncio.run(main_task())    print(f"main END")    return optsif __name__ == '__main__':    main()

Note the subprocess ends very soon, and it seems_stdin_closed isn't properly handled because I see the following output:

3.11.3 (main, Apr  7 2023, 19:25:52) [Clang 14.0.0 (clang-1400.0.29.202)]stdin len: 196428subprocess endedmain ENDFuture exception was never retrievedfuture: <Future finished exception=BrokenPipeError(32, 'Broken pipe')>Traceback (most recent call last):  File "/usr/local/Cellar/python@3.11/3.11.3/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/subprocess.py", line 152, in _feed_stdin    await self.stdin.drain()  File "/usr/local/Cellar/python@3.11/3.11.3/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/streams.py", line 378, in drain    await self._protocol._drain_helper()  File "/usr/local/Cellar/python@3.11/3.11.3/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/streams.py", line 173, in _drain_helper    await waiter  File "/usr/local/Cellar/python@3.11/3.11.3/Frameworks/Python.framework/Versions/3.11/lib/python3.11/asyncio/unix_events.py", line 709, in _write_ready    n = os.write(self._fileno, self._buffer)        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^BrokenPipeError: [Errno 32] Broken pipe

@nullstd
Copy link

cc@1st1

@gpshead
Copy link
Member

gpshead commentedJun 9, 2023•
edited by AlexWaygood
Loading

please do not post questions and cc people on long closed/merged PRs. Please use the discuss.python.org Help category - and only when it seems an actual CPython issue has been found, file a new issue in the issue tracker.

@pythonpython locked asresolvedand limited conversation to collaboratorsJun 9, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@1st11st11st1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@asvetlov@miss-islington@bedevere-bot@nullstd@gpshead@1st1@the-knights-who-say-ni
[8]ページ先頭
©2009-2025 Movatter.jp