Skip to content
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.

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

Using cached_property results in undesired references #1220

Closed
AdrianSosic opened this issue Jan 3, 2024 · 2 comments · Fixed by #1221
Closed

Using cached_property results in undesired references #1220

AdrianSosic opened this issue Jan 3, 2024 · 2 comments · Fixed by #1221

Comments

@AdrianSosic
Copy link

I'm excited to see that the latest attrs version 23.2.0 now supports using cached_property in combination with slotted classes 🥇👍🏼 I was about to enable this feature in my code base but then noticed the following issue: the garbage collection step described here no longer seems to work. I haven't checked how exactly the use of cached_property was enabled, but the applied mechanism seems to keep some references to the original classes and thus messes with the garbage collector, which I think is an undesired side-effect.

Here a minimal example to reproduce the issue:

import gc
from functools import cached_property
from attrs import define

@define
class Base:
    pass

@define
class SubClass(Base):
    @cached_property
    def value(self) -> int:
        return 0

gc.collect()
print(Base.__subclasses__())

With the cached_property decorator, this gives the following output:

[<class '__main__.SubClass'>, <class '__main__.SubClass'>]

while removing it yields the desired result:

[<class '__main__.SubClass'>]
@AdrianSosic
Copy link
Author

Also tagging @diabolo-dan here, who contributed the caching mechanism 🙃

@diabolo-dan
Copy link
Contributor

diabolo-dan commented Jan 3, 2024

I agree that doesn't seem ideal.

It seems like the problem is that the original class gets added to globals when calling _make_method (it's needed to be added to a closure for later substitution).

We can avoid this by either:

  • Using a sentinel value instead of the class itself.
  • passing in the class as a local instead of a global.

I guess that the latter option would make more sense.

I think there may also be a related issue if super() is used inside a cached property (or otherwise a closure to the class is created), as we don't update the closures on them.

I've put up:
#1221

With some possible fixes, though it seems like super/__getattr__ don't interact as I'd expect, so there's still a limitation there.

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 a pull request may close this issue.

2 participants