◐ Shell
reader mode source ↗
Skip to content

GH-98539: fix ref cycle in _SSLProtocolTransport after close #98540

Merged
gvanrossum merged 1 commit into
python:mainfrom
kumaraditya303:ssl-fix
Oct 22, 2022
Merged

GH-98539: fix ref cycle in _SSLProtocolTransport after close #98540
gvanrossum merged 1 commit into
python:mainfrom
kumaraditya303:ssl-fix

Conversation

@kumaraditya303

@kumaraditya303 kumaraditya303 commented Oct 22, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

@kumaraditya303 kumaraditya303 marked this pull request as ready for review October 22, 2022 08:48
@gvanrossum gvanrossum merged commit 62bf5d8 into python:main Oct 22, 2022
@miss-islington

miss-islington commented Oct 22, 2022

Copy link
Copy Markdown
Contributor

Thanks @kumaraditya303 for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 22, 2022
@kumaraditya303 @miss-islington
pythonGH-98539: fix ref cycle in _SSLProtocolTransport after close (p… …
1d222d3 
…ythonGH-98540)

(cherry picked from commit 62bf5d8)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@bedevere-bot

bedevere-bot commented Oct 22, 2022

Copy link
Copy Markdown

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

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Oct 22, 2022
@kumaraditya303 kumaraditya303 deleted the ssl-fix branch October 22, 2022 16:13
@kumaraditya303 kumaraditya303 mentioned this pull request Oct 22, 2022
Closed
gvanrossum pushed a commit that referenced this pull request Oct 24, 2022
@miss-islington @kumaraditya303
[3.11] GH-98539: fix ref cycle in _SSLProtocolTransport after close (… …
bd8b32b 
…GH-98540) (#98551)

GH-98539: fix ref cycle in `_SSLProtocolTransport` after close  (GH-98540)
(cherry picked from commit 62bf5d8)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
pquentin added a commit to pquentin/rally that referenced this pull request May 4, 2023
@pquentin
Downgrade Python 3.11 to work around aiohttp bug …
5e2bd9e 
Since python/cpython#98540 which was included in
Python 3.11.1, aiohttp can fail if two tasks are trying to close the
connection concurrently.
This was referenced May 4, 2023
Merged
Closed
@bdraco bdraco mentioned this pull request May 12, 2023
Closed
1 task
@bdraco bdraco mentioned this pull request May 13, 2023
Merged
20 tasks
@Dreamsorcerer

Dreamsorcerer commented May 13, 2023

Copy link
Copy Markdown
Contributor

This change makes it unsafe to call .abort() after being closed (which seems to be causing exceptions and severe memory leaks for some aiohttp users in 3.11.1+). We'll obviously add a check to aiohttp, but should .abort() be updated to be safe to call?

@Dreamsorcerer Dreamsorcerer mentioned this pull request May 13, 2023
Closed
@gvanrossum

gvanrossum commented May 13, 2023

Copy link
Copy Markdown
Member

This change makes it unsafe to call .abort() after being closed (which seems to be causing exceptions and severe memory leaks for some aiohttp users in 3.11.1+). We'll obviously add a check to aiohttp, but should .abort() be updated to be safe to call?

Makes sense. Can you send a PR?

balloob pushed a commit to home-assistant/core that referenced this pull request May 14, 2023
@bdraco
Disable cleanup_closed for aiohttp.TCPConnector with cpython 3.11.1+ (#… …
7d371a3 
…93013)

* Disable cleanup_closed for aiohttp.TCPConnector with cpython 3.11.2+

There is currently a relatively fast memory leak when using
cpython 3.11.2+ and cleanup_closed with aiohttp

For my production instance it was leaking ~450MiB per day
of `MemoryBIO`, `SSLProtocol`, `SSLObject`, `_SSLProtocolTransport`
`memoryview`, and `managedbuffer` objects

see aio-libs/aiohttp#7252
see python/cpython#98540

* Update homeassistant/helpers/aiohttp_client.py
@Dreamsorcerer Dreamsorcerer mentioned this pull request May 14, 2023
Merged
@Dreamsorcerer

Dreamsorcerer commented May 14, 2023

Copy link
Copy Markdown
Contributor

@gvanrossum #104474

balloob pushed a commit to home-assistant/core that referenced this pull request May 14, 2023
@bdraco @balloob
Disable cleanup_closed for aiohttp.TCPConnector with cpython 3.11.1+ (#… …
13c51e9 
…93013)

* Disable cleanup_closed for aiohttp.TCPConnector with cpython 3.11.2+

There is currently a relatively fast memory leak when using
cpython 3.11.2+ and cleanup_closed with aiohttp

For my production instance it was leaking ~450MiB per day
of `MemoryBIO`, `SSLProtocol`, `SSLObject`, `_SSLProtocolTransport`
`memoryview`, and `managedbuffer` objects

see aio-libs/aiohttp#7252
see python/cpython#98540

* Update homeassistant/helpers/aiohttp_client.py
teharris1 pushed a commit to teharris1/home-assistant that referenced this pull request May 23, 2023
@bdraco @teharris1
Disable cleanup_closed for aiohttp.TCPConnector with cpython 3.11.1+ (h… …
9cde290 
…ome-assistant#93013)

* Disable cleanup_closed for aiohttp.TCPConnector with cpython 3.11.2+

There is currently a relatively fast memory leak when using
cpython 3.11.2+ and cleanup_closed with aiohttp

For my production instance it was leaking ~450MiB per day
of `MemoryBIO`, `SSLProtocol`, `SSLObject`, `_SSLProtocolTransport`
`memoryview`, and `managedbuffer` objects

see aio-libs/aiohttp#7252
see python/cpython#98540

* Update homeassistant/helpers/aiohttp_client.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes

@1st1 1st1 Awaiting requested review from 1st1

@asvetlov asvetlov Awaiting requested review from asvetlov

Assignees

No one assigned

Labels

skip news topic-asyncio

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kumaraditya303 @miss-islington @bedevere-bot @Dreamsorcerer @gvanrossum