Skip to content

gh-116946: fully implement GC protocol for _hashlib objects #138289

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 1, 2025
Merged

gh-116946: fully implement GC protocol for _hashlib objects #138289

merged 3 commits into from
Sep 1, 2025
+41 −10

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Aug 31, 2025 •
edited by bedevere-app bot
Loading

I noticed that HASHXOF didn't explicitly specify its deallocation function because HASHXOF is inherting HASH (it's constructed using PyType_FromSpecWithBases).

@picnixz picnixz requested a review from gpshead as a code owner August 31, 2025 11:16
@bedevere-app bedevere-app bot mentioned this pull request Aug 31, 2025
Open
@picnixz picnixz marked this pull request as draft August 31, 2025 12:33
@picnixz picnixz marked this pull request as ready for review August 31, 2025 12:57
@picnixz picnixz requested a review from ZeroIntensity August 31, 2025 13:20
ZeroIntensity
ZeroIntensity approved these changes Aug 31, 2025
View reviewed changes
Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

Thanks again!

@gpshead gpshead added needs backport to 3.14 bugs and security fixes 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section labels Aug 31, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 23c4d91 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F138289%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Aug 31, 2025
@picnixz picnixz merged commit 6f1dd95 into python:main Sep 1, 2025
66 checks passed
@miss-islington-app
Copy link

Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@picnixz picnixz deleted the fix/gc/hashlib-heap-types-116946 branch September 1, 2025 08:23
@miss-islington-app
Copy link

Sorry, @picnixz, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 6f1dd9551a69c8c76d066a04e94db6dbc6c7597c 3.14

picnixz added a commit to picnixz/cpython that referenced this pull request Sep 1, 2025
@picnixz
[3.14] pythongh-116946: fully implement GC protocol for _hashlib ob…
459cc57 
…jects (pythonGH-138289)

(cherry picked from commit 6f1dd95)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Sep 1, 2025

GH-138324 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Sep 1, 2025
picnixz added a commit to picnixz/cpython that referenced this pull request Sep 1, 2025
@picnixz
[3.13] pythongh-116946: fully implement GC protocol for _hashlib ob…
0294a0b 
…jects (pythonGH-138289)

(cherry picked from commit 6f1dd95)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Sep 1, 2025

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

picnixz added a commit to picnixz/cpython that referenced this pull request Sep 1, 2025
@picnixz
[3.14] pythongh-116946: fully implement GC protocol for _hashlib ob…
10c77d7 
…jects (pythonGH-138289)

(cherry picked from commit 6f1dd95)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz added a commit to picnixz/cpython that referenced this pull request Sep 1, 2025
@picnixz
[3.13] pythongh-116946: fully implement GC protocol for _hashlib ob…
910bd27 
…jects (pythonGH-138289)

(cherry picked from commit 6f1dd95)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz added a commit to picnixz/cpython that referenced this pull request Sep 1, 2025
@picnixz
[3.14] pythongh-116946: fully implement GC protocol for _hashlib ob…
adbb8b6 
…jects (pythonGH-138289)

(cherry picked from commit 6f1dd95)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz added a commit that referenced this pull request Sep 1, 2025
@picnixz
[3.13] gh-116946: fully implement GC protocol for _hashlib objects (G…
21b5932 
…H-138289) (#138326)

(cherry picked from commit 6f1dd95)
@picnixz
Copy link
Member Author

picnixz commented Sep 1, 2025 •
edited
Loading

@kumaraditya303 If the type is both immutable and disallows instantiation BUT can be inherited, should I still implement the GC protocol? If so, I need to only keep half of the PR. Otherwise I can fully revert it.

@kumaraditya303
Copy link
Contributor

kumaraditya303 commented Sep 1, 2025 •
edited
Loading

if the type is both immutable and disallows instantiation BUT can be inherited, should I still implement the GC protocol? If so, I need to only keep half of the PR. Otherwise I can fully revert it.

No, the subclass would implement the GC protocol if it needs to, it is not needed for base type in this case as it is immutable and objects are empty.

@picnixz picnixz mentioned this pull request Sep 1, 2025
Closed
picnixz added a commit to picnixz/cpython that referenced this pull request Sep 1, 2025
@picnixz
Revert "pythongh-116946: fully implement GC protocol for _hashlib o…
dc8f5a0 
…bjects (python#138289)"

This reverts commit 6f1dd95.
picnixz added a commit to picnixz/cpython that referenced this pull request Sep 1, 2025
@picnixz
Revert "[3.13] pythongh-116946: fully implement GC protocol for `_has…
586c906 
…hlib` objects (pythonGH-138289) (python#138326)"

This reverts commit 21b5932.
kumaraditya303 pushed a commit that referenced this pull request Sep 1, 2025
@picnixz
[3.13] gh-116946: Revert GC protocol for immutable empty heap types (G…
a38f026 
…H-138322, GH-138323, GH-138326) (#138337)

* Revert "[3.13] gh-116946: fully implement GC protocol for `bz2` objects (GH-138266) (#138322)"

This reverts commit 90036f5.

* Revert "[3.13] gh-116946: fully implement GC protocol for `lzma` objects (GH-138288) (#138323)"

This reverts commit 828682d.

* Revert "[3.13] gh-116946: fully implement GC protocol for `_hashlib` objects (GH-138289) (#138326)"

This reverts commit 21b5932.
kumaraditya303 pushed a commit that referenced this pull request Sep 1, 2025
@picnixz
gh-116946: Revert GC protocol for immutable empty heap types (GH-138266…
c9b252c 
…, GH-138288, GH-138289) (#138338)

* Revert "gh-116946: fully implement GC protocol for `bz2` objects (#138266)"

This reverts commit 9be91f6.

* Revert "gh-116946: fully implement GC protocol for `lzma` objects (#138288)"

This reverts commit 3ea16f9.

* Revert "gh-116946: fully implement GC protocol for `_hashlib` objects (#138289)"

This reverts commit 6f1dd95.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gpshead gpshead gpshead approved these changes

@ZeroIntensity ZeroIntensity ZeroIntensity approved these changes

+1 more reviewer

@auvipy auvipy auvipy approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@picnixz picnixz

Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@picnixz @bedevere-bot @kumaraditya303 @gpshead @auvipy @ZeroIntensity