Skip to content

gh-130179: Fix some references leaks in _pickle#130180

Merged
sobolevn merged 2 commits into
python:mainfrom
sergey-miryanov:gh-130179-pickle-memory-leaks
Feb 16, 2025
Merged

gh-130179: Fix some references leaks in _pickle#130180
sobolevn merged 2 commits into
python:mainfrom
sergey-miryanov:gh-130179-pickle-memory-leaks

Conversation

@sergey-miryanov

@sergey-miryanov sergey-miryanov commented Feb 16, 2025

Copy link
Copy Markdown
Contributor

I found some possible memory leaks in _pickle. This PR fixes it.

@sergey-miryanov

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka Please take a look. It seems related to your changes.

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indeed, it looks like an oversight.

@sobolevn sobolevn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@sobolevn

Copy link
Copy Markdown
Member

Refs #125752

@sobolevn sobolevn added the needs backport to 3.13 bugs and security fixes label Feb 16, 2025
@picnixz picnixz enabled auto-merge (squash) February 16, 2025 13:02
@picnixz picnixz disabled auto-merge February 16, 2025 13:03
@picnixz picnixz enabled auto-merge (squash) February 16, 2025 13:05
@picnixz

picnixz commented Feb 16, 2025

Copy link
Copy Markdown
Member

(I disabled/enabled the auto-merge because I had a typo in the commit message and it's not possible to edit it after hitting the auto-merge button)

@picnixz

picnixz commented Feb 16, 2025

Copy link
Copy Markdown
Member

Oh the ARM build is failing with the usual network error. I'll re-disable the auto-merge just in case we need to re-update the PR later.

@picnixz picnixz disabled auto-merge February 16, 2025 13:09

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. 👍

@sobolevn sobolevn merged commit e7f00cd into python:main Feb 16, 2025
@miss-islington-app

Copy link
Copy Markdown

Thanks @sergey-miryanov for the PR, and @sobolevn for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 16, 2025
…_pickle` (pythonGH-130180)

(cherry picked from commit e7f00cd)

Co-authored-by: Sergey Miryanov <sergey.miryanov@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@bedevere-app

bedevere-app Bot commented Feb 16, 2025

Copy link
Copy Markdown

GH-130190 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Feb 16, 2025
sobolevn pushed a commit that referenced this pull request Feb 16, 2025
…`_pickle` (GH-130180) (#130190)

gh-130179: Fix `persistent_{id,load}_attr` reference leaks in `_pickle` (GH-130180)
(cherry picked from commit e7f00cd)

Co-authored-by: Sergey Miryanov <sergey.miryanov@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants