gh-135228: When @dataclass(slots=True) replaces a dataclass, make the original class collectible (take 2) #137047
Conversation
…ke the original class collectible (python#136893) An interesting hack, but more localized in scope than python#135230. This may be a breaking change if people intentionally keep the original class around when using `@dataclass(slots=True)`, and then use `__dict__` or `__weakref__` on the original class. Co-authored-by: Alyssa Coghlan <[email protected]>
JelleZijlstra
requested review from
ericvsmith and
ericsnowcurrently
as code owners
| if (PyDict_PopString(dict, "__weakref__", NULL) < 0) { | ||
| return NULL; | ||
| } | ||
| PyType_Modified(typeobj); |
There was a problem hiding this comment.
This seems to be technically not thread-safe, because somebody in another thread may be working with the type between the PopString calls and the PyType_Modified call. However, this is also true for the various functions in typeobject.c that modify the type object.
There was a problem hiding this comment.
Could anything be deallocated as a result of these pops? Could have a reentrancy problem even without threads present.
There was a problem hiding this comment.
I guess to prevent that we can hold onto a reference to the old value (by passing the third arg to PyDict_PopString) and DECREF it only after we call PyType_Modified.
Maybe we can check that it's a heap type?
I'm not aware of other descriptors that could cause problems. It's better to minimize the amount of changes we make to the class, because there are edge cases where the class is still accessible after the dataclass transformation is applied. |
This is a redo of #136893 without relying on a hack to get to the type dictionary.