Skip to content

Conversation

@miss-islington
Copy link
Contributor

@miss-islingtonmiss-islington commented Aug 5, 2023

(cherry picked from commit 41178e4)

Co-authored-by: Kumar Aditya kumaraditya@python.org

…is not closed (pythonGH-107650) (cherry picked from commit 41178e4) Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@Yhg1sYhg1s enabled auto-merge (squash) August 5, 2023 12:41
@Yhg1sYhg1s disabled auto-merge August 5, 2023 12:42
@Yhg1s
Copy link
Member

Yhg1s commented Aug 5, 2023

I think this should go into rc2, but without the warning. I think the warning is too potentially disruptive this late in the release cycle.

@kumaraditya303kumaraditya303 changed the title [3.12] GH-106684: raise ResourceWarning when asyncio.StreamWriter is not closed (GH-107650)[3.12] GH-106684: Close asyncio.StreamWriter when asyncio.StreamWriter is not closed by application (GH-107650)Aug 10, 2023
@kumaraditya303kumaraditya303 added the needs backport to 3.11 only security fixes label Aug 10, 2023
@Yhg1sYhg1s merged commit 7853c76 into python:3.12Aug 10, 2023
@miss-islington
Copy link
ContributorAuthor

Thanks @miss-islington for the PR, and @Yhg1s for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islingtonmiss-islington deleted the backport-41178e4-3.12 branch August 10, 2023 09:24
@bedevere-bot
Copy link

GH-107836 is a backport of this pull request to the 3.11 branch.

miss-islington added a commit to miss-islington/cpython that referenced this pull request Aug 10, 2023
…reamWriter` is not closed by application (pythonGH-107650) (pythonGH-107656) pythonGH-106684: raise `ResourceWarning` when `asyncio.StreamWriter` is not closed (pythonGH-107650) (cherry picked from commit 41178e4) (cherry picked from commit 7853c76) Co-authored-by: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> Co-authored-by: Kumar Aditya <kumaraditya@python.org> Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@bedevere-botbedevere-bot removed the needs backport to 3.11 only security fixes label Aug 10, 2023
kumaraditya303 added a commit that referenced this pull request Aug 10, 2023
…treamWriter` is not closed by application (GH-107650) (GH-107656) (#107836) [3.12] GH-106684: Close `asyncio.StreamWriter` when `asyncio.StreamWriter` is not closed by application (GH-107650) (GH-107656) GH-106684: raise `ResourceWarning` when `asyncio.StreamWriter` is not closed (GH-107650) (cherry picked from commit 41178e4) (cherry picked from commit 7853c76) Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@rigens
Copy link

It seems that closing the connection in the StreamWriter's __del__ method is not a good idea. Consider the following use case

importasyncioHOST='ifconfig.me'PORT=80asyncdefconnect() ->asyncio.Transport: reader, writer=awaitasyncio.open_connection( host=HOST, port=PORT, ) returnwriter.transportasyncdeffetch(): loop=asyncio.get_running_loop() transport=awaitconnect() # !!! transport is already closed here after this PR mergedreader=asyncio.StreamReader(limit=2**16, loop=loop) protocol=asyncio.StreamReaderProtocol(reader, loop=loop) transport.set_protocol(protocol) loop.call_soon(protocol.connection_made, transport) loop.call_soon(transport.resume_reading) writer=asyncio.StreamWriter( transport=transport, protocol=protocol, reader=reader, loop=loop, ) request=f'GET /ip HTTP/1.1\r\nHost: {HOST}\r\nConnection: close\r\n\r\n'.encode() writer.write(request) awaitwriter.drain() response=awaitreader.read(-1) print(response) if__name__=='__main__': asyncio.run(fetch())

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bugAn unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@miss-islington@Yhg1s@bedevere-bot@rigens@kumaraditya303