GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-98539: fix ref cycle in `_SSLProtocolTransport` after close by kumaraditya303 · Pull Request #98540 · python/cpython

Conversation

@kumaraditya303

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: _SSLProtocolTransport keeps reference to protocol after close #98539

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added the awaiting review label Oct 22, 2022
@kumaraditya303 kumaraditya303 added skip news needs backport to 3.11 only security fixes labels Oct 22, 2022
@kumaraditya303
fix ref cycle
39a0df2
@kumaraditya303
kumaraditya303 force-pushed the ssl-fix branch from 7cbbcf4 to 39a0df2 Compare October 22, 2022 07:44
@kumaraditya303
kumaraditya303 marked this pull request as ready for review October 22, 2022 08:48
@kumaraditya303
kumaraditya303 requested review from 1st1, asvetlov and gvanrossum as code owners October 22, 2022 08:48
gvanrossum
gvanrossum reviewed Oct 22, 2022
View reviewed changes
Comment thread Lib/asyncio/sslproto.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303
kumaraditya303 requested a review from gvanrossum October 22, 2022 15:59
gvanrossum
gvanrossum approved these changes Oct 22, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Oct 22, 2022
@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.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting merge label Oct 22, 2022
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 <[email protected]>
@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
_SSLProtocolTransport keeps reference to protocol after close #98539
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 <[email protected]>
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
Downgrade Python 3.11 to work around aiohttp bug elastic/rally#1712
Merged
AttributeError: 'NoneType' object has no attribute '_abort' on aiohttp connection close elastic/rally#1714
Closed
@bdraco bdraco mentioned this pull request May 12, 2023
Memory leak with Python 3.11.2 aio-libs/aiohttp#7252
Closed
1 task done
bdraco added a commit to home-assistant/core that referenced this pull request May 13, 2023
@bdraco
Disable cleanup_closed for aiohttp.TCPConnector with cpython 3.11.2+
2cf8ade 
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
@bdraco bdraco mentioned this pull request May 13, 2023
Disable cleanup_closed for aiohttp.TCPConnector with cpython 3.11.1+ home-assistant/core#93013
Merged
2 of 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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Dreamsorcerer Dreamsorcerer mentioned this pull request May 13, 2023
Fix unsafe .abort() call aio-libs/aiohttp#7280
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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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
gh-98539: Make _SSLTransportProtocol.abort() safe to call when closed #104474
Merged
@Dreamsorcerer

Dreamsorcerer commented May 14, 2023

Copy link
Copy Markdown
Contributor

@gvanrossum #104474
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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
@kumaraditya303 kumaraditya303 added the topic-asyncio label Jul 7, 2025
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

Uh oh!

There was an error while loading. Please reload this page.

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.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@kumaraditya303 @miss-islington @bedevere-bot @Dreamsorcerer @gvanrossum
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.