Skip to content

Conversation

@Prometheus3375
Copy link
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.

More details in these comments: 1, 2.

Any help with tests for the issue would be appreciated. I guess just checking that __setattr__ and __delattr__ reference the new class would be sufficient.

@python-cla-bot
Copy link

python-cla-bot bot commented Jan 19, 2026

All commit authors signed the Contributor License Agreement.

CLA signed

@JelleZijlstra
Copy link
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
Copy link
Contributor Author

Prometheus3375 commented Jan 19, 2026

Added 3 tests:

  1. That the original class is gc-ed if it is a frozen slotted dataclass.
  2. That methods added by frozen=True reference the new class via __class__.
  3. That methods added by frozen=True do not reference the old class.

I will add news a bit later.

Copy link
Contributor Author

@Prometheus3375 Prometheus3375 left a comment

Choose a reason for hiding this comment

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

By the way, 3.12 also suffers from the same issue. I do not have 3.13, but I guess there it is also present. This PR should be backported to 3.13.

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.

2 participants