classification
Title: Fix possible leaks in close methods
Type: resource usage Stage: resolved
Components: Library (Lib) Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: martin.panter, mitya57, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-04-04 11:13 by serhiy.storchaka, last changed 2015-05-06 06:54 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
close.patch serhiy.storchaka, 2015-04-04 11:12 review
Messages (7)
msg240063 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-04 11:12
Proposed patch fixes two related issues in a number of modules.

1. close() methods sometimes release multiple resources. Every closing operation can fail, but it shouldn't prevent releasing other resources. See for example issue21802.

2. close() should be idempotent. I.e. calling close() second times shouldn't have any effect. Even if close() failed, repeated call of close() (usually in __exit__(), in __del__(), or in finally block) shouldn't raise an exception.

Many close() methods already satisfy these conditions, but not all.
msg240245 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-04-08 02:38
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.
msg240255 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-04-08 10:44
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.
msg240289 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-04-08 22:11
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.
msg240413 - (view) Author: Roundup Robot (python-dev) Date: 2015-04-10 10:30
New changeset f7ddec2e9e93 by Serhiy Storchaka in branch '2.7':
Issue #23865: close() methods in multiple modules now are idempotent and more
https://hg.python.org/cpython/rev/f7ddec2e9e93

New changeset e826940911c8 by Serhiy Storchaka in branch '3.4':
Issue #23865: close() methods in multiple modules now are idempotent and more
https://hg.python.org/cpython/rev/e826940911c8

New changeset 4ddec11b5faf by Serhiy Storchaka in branch 'default':
Issue #23865: close() methods in multiple modules now are idempotent and more
https://hg.python.org/cpython/rev/4ddec11b5faf
msg242580 - (view) Author: Dmitry Shachnev (mitya57) * Date: 2015-05-04 20:37
This broke docutils, see issue #24125.
msg242645 - (view) Author: Roundup Robot (python-dev) Date: 2015-05-06 06:54
New changeset fe340c2a220e by Serhiy Storchaka in branch '2.7':
Issue #24125: Saved error's line and column numbers when an error is occured
https://hg.python.org/cpython/rev/fe340c2a220e

New changeset 7f8cd879687b by Serhiy Storchaka in branch '3.4':
Issue #24125: Saved error's line and column numbers when an error is occured
https://hg.python.org/cpython/rev/7f8cd879687b

New changeset 1fb83fa2cdef by Serhiy Storchaka in branch 'default':
Issue #24125: Saved error's line and column numbers when an error is occured
https://hg.python.org/cpython/rev/1fb83fa2cdef
History
Date User Action Args
2015-05-06 06:54:24python-devsetmessages: + msg242645
2015-05-04 20:37:52mitya57setnosy: + mitya57
messages: + msg242580
2015-04-10 17:15:50serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2015-04-10 10:30:56python-devsetnosy: + python-dev
messages: + msg240413
2015-04-08 22:11:18martin.pantersetmessages: + msg240289
2015-04-08 10:44:59serhiy.storchakasetmessages: + msg240255
2015-04-08 02:38:15martin.pantersetnosy: + martin.panter
messages: + msg240245
2015-04-04 11:13:00serhiy.storchakacreate