Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.12] GH-106684: Close asyncio.StreamWriter when asyncio.StreamWriter is not closed by application (GH-107650) #107656

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

miss-islington
Copy link
Contributor

@miss-islington miss-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>
@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.

@kumaraditya303 kumaraditya303 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
@kumaraditya303 kumaraditya303 added the needs backport to 3.11 only security fixes label Aug 10, 2023
@Yhg1s Yhg1s merged commit 7853c76 into python:3.12 Aug 10, 2023
19 checks passed
@miss-islington
Copy link
Contributor Author

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-islington miss-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-bot bedevere-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

rigens commented Sep 12, 2023

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

import asyncio

HOST = 'ifconfig.me'
PORT = 80


async def connect() -> asyncio.Transport:
    reader, writer = await asyncio.open_connection(
        host=HOST,
        port=PORT,
    )
    return writer.transport


async def fetch():
    loop = asyncio.get_running_loop()

    transport = await connect()
    # !!! transport is already closed here after this PR merged

    reader = 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)
    await writer.drain()

    response = await reader.read(-1)
    print(response)


if __name__ == '__main__':
    asyncio.run(fetch())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants