Skip to content

gh-105936: Properly update closure cells for __setattr__ and __delattr__ in frozen dataclasses with slots#144021

Merged
gpshead merged 14 commits intopython:mainfrom
Prometheus3375:fix-dataclass-frozen-slots-leak
Apr 12, 2026
Merged

gh-105936: Properly update closure cells for __setattr__ and __delattr__ in frozen dataclasses with slots#144021
gpshead merged 14 commits intopython:mainfrom
Prometheus3375:fix-dataclass-frozen-slots-leak

Conversation

@Prometheus3375
Copy link
Copy Markdown
Contributor

@Prometheus3375 Prometheus3375 commented Jan 19, 2026

Specifying frozen=True for a dataclass adds methods __setattr__ and __detattr__ to it. These methods reference the class via closure. If slots=True is also present, then the class is recreated with slots with all methods being copied. Notably, every method referencing the old class via closure in variable __class__ is properly updated, but __setattr__ and __detattr__ reference the old class via cls; hence, they are still referencing the old class instead of the new one. This PR fixes this issue.

Additional details:

The issue is present in Python as early as in 3.10. This PR must be backported to 3.14 and possibly 3.13.

Originally this PR was referencing gh-135228, but soon I realized that the issue is not connected to regression in 3.14. There is an older PR #105937 that fixes the issue, but it is heavily outdated.

@python-cla-bot
Copy link
Copy Markdown

python-cla-bot bot commented Jan 19, 2026

All commit authors signed the Contributor License Agreement.

CLA signed

@JelleZijlstra
Copy link
Copy Markdown
Member

Thanks! For the test you can do something like that in the original fix, https://github.com/python/cpython/pull/137047/files#diff-212e368b34eb9b134f87e765787d6d26b747235a358e13da8922f83861c0d676 .

@Prometheus3375 Prometheus3375 force-pushed the fix-dataclass-frozen-slots-leak branch from fdc81bb to a6de29b Compare January 19, 2026 06:19
@Prometheus3375

This comment was marked as outdated.

Prometheus3375

This comment was marked as outdated.

@Prometheus3375

This comment was marked as outdated.

@Prometheus3375 Prometheus3375 changed the title gh-135228: Properly update closure cells for __setattr__ and __detattr__ in frozen dataclasses with slots gh-105936: Properly update closure cells for __setattr__ and __detattr__ in frozen dataclasses with slots Jan 22, 2026
@Prometheus3375
Copy link
Copy Markdown
Contributor Author

While this PR is not merged, should AKCS be updated?

@JelleZijlstra
Copy link
Copy Markdown
Member

If I remember correctly the latest policy on Misc/ACKS is that people can add themselves if they want but it's not required. The file is a pain to keep fully correct and sort of redundant with git log, so there's been talk of getting rid of it but not sure anything concrete has changed yet.

Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM as well.

cc @ericvsmith @carljm

@gpshead gpshead changed the title gh-105936: Properly update closure cells for __setattr__ and __detattr__ in frozen dataclasses with slots gh-105936: Properly update closure cells for __setattr__ and __delattr__ in frozen dataclasses with slots Apr 12, 2026
@gpshead gpshead added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Apr 12, 2026
Copy link
Copy Markdown
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

Thanks!

@gpshead gpshead self-assigned this Apr 12, 2026
@gpshead gpshead merged commit 8a398bf into python:main Apr 12, 2026
55 checks passed
@miss-islington-app
Copy link
Copy Markdown

Thanks @Prometheus3375 for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 12, 2026
…`__delattr__` in frozen dataclasses with slots (pythonGH-144021)

(cherry picked from commit 8a398bf)

Co-authored-by: Sviataslau <35541026+Prometheus3375@users.noreply.github.com>
@miss-islington-app
Copy link
Copy Markdown

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

cherry_picker 8a398bfbbc6769f6cabb3177702e7a506e203d61 3.13

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Apr 12, 2026

GH-148469 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 Apr 12, 2026
gpshead pushed a commit to miss-islington/cpython that referenced this pull request Apr 12, 2026
…`__delattr__` in frozen dataclasses with slots (pythonGH-144021)

(cherry picked from commit 8a398bf)

Co-authored-by: Sviataslau <35541026+Prometheus3375@users.noreply.github.com>
gpshead pushed a commit that referenced this pull request Apr 12, 2026
… `__delattr__` in frozen dataclasses with slots (GH-144021) (#148469)

gh-105936: Properly update closure cells for `__setattr__` and `__delattr__` in frozen dataclasses with slots (GH-144021)
(cherry picked from commit 8a398bf)

Co-authored-by: Prometheus3375 <prometheus3375@gmail.com>
Co-authored-by: Sviataslau <35541026+Prometheus3375@users.noreply.github.com>
gpshead added a commit to gpshead/cpython that referenced this pull request Apr 12, 2026
…_` and `__delattr__` in frozen dataclasses with slots (pythonGH-144021)

pythongh-105936: Properly update closure cells for `__setattr__` and `__delattr__` in frozen dataclasses with slots (pythonGH-144021)
(cherry picked from commit 8a398bf)

The cherry-pick required additional changes beyond the original commit
because 3.13 lacks the `__class__` closure cell fixup machinery that
was added in 3.14 by pythonGH-124455 (pythongh-90562). Specifically:

- Backported `_update_func_cell_for__class__()` helper function and the
  closure fixup loop in `_add_slots()` from pythonGH-124455. Without these,
  renaming the closure variable from `cls` to `__class__` has no effect
  because nothing updates the cell when the class is recreated with slots.
- Changed `_add_slots()` to use `newcls` instead of reusing `cls` for the
  recreated class, so both old and new class references are available for
  the fixup loop.
- Replaced `assertNotHasAttr` with `assertFalse(hasattr(...))` in tests
  (assertNotHasAttr was added in 3.14).
- Dropped `test_original_class_is_gced` additions (that test does not
  exist on 3.13; it was added by pythonGH-137047 for pythongh-135228 which was not
  backported to 3.13).

Co-authored-by: Prometheus3375 <prometheus3375@gmail.com>
Co-authored-by: Sviataslau <35541026+Prometheus3375@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Apr 12, 2026

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants