Skip to content

gh-116946: fully implement GC protocol for lzma objects #138288

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 4 commits into from
Sep 1, 2025
Merged

gh-116946: fully implement GC protocol for lzma objects #138288

merged 4 commits into from
Sep 1, 2025

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Aug 31, 2025 •
edited
Loading

Outdated conversation

@ZeroIntensity Using tp_alloc directly feels wrong to me even because it's about relying on implementation details that are not documented (and not explicitly recommended by our docs recommend), while using PyObject_GC_New forces me to actually memset the structure (with an additional offset!) as otherwise I need to initialize the fields manually (this is essentially to prevent a segfault when invoking the (external) LZMA interface).

So I really think we should have some PyObject_GC_New-like function that zeroes the remaining fields or a just a function for zeroing the rest of a PyObject fields, that is:

void _PyObject_MemSetZero(PyObject *self, size_t basicsize) {
    assert(self != NULL);
    const size_t offset = sizeof(struct { PyObject_HEAD });
    assert(basicsize >= offset);
    memset((char *)self + offset, 0, basicsize - offset);
}

and that we also have a private constant indicating the size of PyObject_HEAD. WDYT?

EDIT: I was looking at the 3.13 docs but the recommendations were given in the 3.14 docs, which is why I missed them.

@bedevere-app bedevere-app bot mentioned this pull request Aug 31, 2025
Open
@ZeroIntensity

This comment was marked as resolved.

Sign in to view
picnixz
picnixz commented Aug 31, 2025
View reviewed changes
@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 13:06
@picnixz picnixz merged commit 3ea16f9 into python:main Sep 1, 2025
49 checks passed
@picnixz picnixz deleted the fix/gc/lzma-heap-types-116946 branch September 1, 2025 08:22
@picnixz picnixz added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Sep 1, 2025
@miss-islington-app
Copy link

Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

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

cherry_picker 3ea16f990f81e1e3b2892f1dfd213937b1df2a68 3.13

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 1, 2025
@picnixz @miss-islington
pythongh-116946: fully implement GC protocol for lzma objects (pyth…
3dd0b0e 
…onGH-138288)

(cherry picked from commit 3ea16f9)

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-138321 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 lzma objects (
cdfb670 
pythonGH-138288)

(cherry picked from commit 3ea16f9)

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-138323 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 Sep 1, 2025
picnixz added a commit that referenced this pull request Sep 1, 2025
@picnixz
[3.13] gh-116946: fully implement GC protocol for lzma objects (GH-…
828682d 
…138288) (#138323)

(cherry picked from commit 3ea16f9)
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 `lzma…
505032c 
…` objects (pythonGH-138288) (python#138323)"

This reverts commit 828682d.
picnixz added a commit to picnixz/cpython that referenced this pull request Sep 1, 2025
@picnixz
Revert "pythongh-116946: fully implement GC protocol for lzma objec…
fa86d1d 
…ts (python#138288)"

This reverts commit 3ea16f9.
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.
lkollar pushed a commit to lkollar/cpython that referenced this pull request Sep 9, 2025
@picnixz @lkollar
pythongh-116946: fully implement GC protocol for lzma objects (pyth…
a511333 
…on#138288)
lkollar pushed a commit to lkollar/cpython that referenced this pull request Sep 9, 2025
@picnixz @lkollar
pythongh-116946: Revert GC protocol for immutable empty heap types (p…
a6c052e 
…ythonGH-138266, pythonGH-138288, pythonGH-138289) (python#138338)

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

This reverts commit 9be91f6.

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

This reverts commit 3ea16f9.

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

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

@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.
3 participants
@picnixz @ZeroIntensity @auvipy