-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
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
Fix possible leaks in close methods #68053
Comments
Proposed patch fixes two related issues in a number of modules.
Many close() methods already satisfy these conditions, but not all. |
The change to aifc shouldn’t be needed, unless the underlying file object’s close() is not idempotent. And if that is the case, you’re fixing the bug in the wrong place :) Most of the other changes look good from a rough review, though I was only familiar enough with the gzip and HTTP modules to review them thoroughly. I left a few comments on Reitveld. |
Ah, yet one purpose of the patch is to make close() methods more robust when called at shutdown. Objects can be partially deconstructed at that time, attributes can be set to None to break reference loops. That is why I use None as signaling value and always check some attribute for None. The changes to aifc make sense. close() is called from __exit__(). When context manager is used in a generator, it creates a reference loop. So self.file is None in this case and self.file.close() will raise an AttributeError. |
Okay, I think I get it. You are setting some attributes to None, in case they happen to be causing a reference cycle. When close() is called, they are no longer needed, so it might help to breaking potential reference cycles. I have often wondered why this was done. I always assumed it was a carry-over from old code relying on the garbage collector to automatically close files, etc. |
New changeset f7ddec2e9e93 by Serhiy Storchaka in branch '2.7': New changeset e826940911c8 by Serhiy Storchaka in branch '3.4': New changeset 4ddec11b5faf by Serhiy Storchaka in branch 'default': |
This broke docutils, see issue bpo-24125. |
New changeset fe340c2a220e by Serhiy Storchaka in branch '2.7': New changeset 7f8cd879687b by Serhiy Storchaka in branch '3.4': New changeset 1fb83fa2cdef by Serhiy Storchaka in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: