-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-44310: added docs to functools to warn for memory leaks #26528
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
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds sensible to especially document but I will also ping @pablogsal to confirm if this is a bug that needs solving or its acceptable behavior, so to speak.
Thanks a lot @nanjekyejoannah for checking with me. @wouterdb I think it makes sense to mention this in the docs, but I would like to change the phrasing a bit |
Ok, thank you for looking into this so quickly. I agree that the behavior is fully in-line with the rest of the python sdk (if a bit unexpected here) and that my proposed changes are not as well put as they could. Do I understand correctly that this is now in the python team's capable hands and that it would not help for me to propose further changes? |
@@ -144,6 +144,11 @@ The :mod:`functools` module defines the following functions: | |||
functions with side-effects, functions that need to create distinct mutable | |||
objects on each call, or impure functions such as time() or random(). | |||
|
|||
The LRU cache takes string references to the arguments provided in the decorated callable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LRU cache takes string references to the arguments provided in the decorated callable | |
The LRU cache takes strong references to the arguments provided in the decorated callable |
@@ -144,6 +144,11 @@ The :mod:`functools` module defines the following functions: | |||
functions with side-effects, functions that need to create distinct mutable | |||
objects on each call, or impure functions such as time() or random(). | |||
|
|||
The LRU cache takes string references to the arguments provided in the decorated callable | |||
as well as the return value. Notice that this means that if a class method is decorated, the cache | |||
will keep strong references to all arguments provided, including the instance itself (provided as the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will keep strong references to all arguments provided, including the instance itself (provided as the | |
will keep strong references to all arguments provided, including the instance itself (provided as first the |
Closing in favor of PR 26715 |
I implemented the easier fix for https://bugs.python.org/issue44310: documenting the unexpected behavior.
To actually change the behavior look a bit too tricky. WeakKeyDictionary does what is needed, but it doesn't update the LRU linked list, and offers no hook to do so. I could make an attempt to make the change, but I'm not sufficiently familiar with the python codebase to do it without confirmation that I can.
https://bugs.python.org/issue44310