classification
Title: More robust TemporaryDirectory cleanup
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: georg.brandl, haypo, ncoghlan, pitrou, python-dev, sbt, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2013-09-23 14:58 by serhiy.storchaka, last changed 2014-01-27 09:52 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
tempfile_tempdir_cleanup.patch serhiy.storchaka, 2014-01-09 21:54 More robust rmdir review
tempfile_tempdir_cleanup_weakref_finalize.patch serhiy.storchaka, 2014-01-09 22:19 Use weakref.finalize() review
tempdir_finalize.patch pitrou, 2014-01-09 23:01 review
tempfile_tempdir_cleanup_2.patch serhiy.storchaka, 2014-01-10 10:21 review
tempdir_cleanup-3.3.patch serhiy.storchaka, 2014-01-24 17:58 review
tempdir_finalize-3.4.patch serhiy.storchaka, 2014-01-24 17:58 review
Messages (14)
msg198321 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-09-23 14:58
The proposed patch solves the problem with TemporaryDirectory cleanup. If shutil.rmtree() failed because module globals are set to None, TemporaryDirectory now uses own rmtree implementation which does not depends from globals. The patch also fixes other minor problems related to __del__ and fixes do_create() in tests (it deleted subdirectories just after creation).

See also issue10188.
msg198414 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2013-09-25 20:07
An alternative would be to use weakref.finalize() which would guarantee that cleanup happens before any purging occurs.  That would allow the use of shutil:

class TemporaryDirectory(object):
    def __init__(self, suffix="", prefix=template, dir=None):
        self.name = mkdtemp(suffix, prefix, dir)
        self._cleanup = weakref.finalize(self, shutil.rmtree, self.name)

    def __enter__(self):
        return self.name

    def __exit__(self, exc, value, tb):
        self._cleanup()

    def cleanup(self):
        self._cleanup()
msg198422 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-09-25 21:06
Sounds good to me!
msg207798 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-09 21:54
Updated patch to tip.
msg207799 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-09 22:04
An alternative with weakref.finalize() looks very attractive, but unfortunately tests are failed with it.
msg207801 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-09 22:19
Here is a patch which implements Richard's suggestion. test_del_on_shutdown and test_warnings_on_cleanup are failed, but perhaps they are wrong.
msg207803 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-01-09 22:56
Note the finalize-based patch can only work on 3.4.
msg207804 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-01-09 23:01
Here is a working patch for the finalize-based approach.
msg207842 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-10 10:21
Excellent, Antoine!

Here is a patch for 3.3 with tests based on Antoine's patch. My changes to tests:

* TemporaryDirectory instance is preserved as attributes of several modules. So some modules can be destroyed before cleaning up temporary directory.

* Resource warning is not checked because the warning module can be destroyed at time of cleaning up temporary directory.
msg209105 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-24 17:58
Here are updated patches for 3.3 and 3.4. Changed tests for 3.4, a patch for 
3.3 is changed more. Unfortunately in 3.3 exceptions still can be raised when 
try to emit a warning during shutdown. Is there any way to determine the 
shutdown mode?
msg209363 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-01-26 22:53
> Here are updated patches for 3.3 and 3.4. Changed tests for 3.4, a patch for 
> 3.3 is changed more. Unfortunately in 3.3 exceptions still can be raised when 
> try to emit a warning during shutdown. Is there any way to determine the 
> shutdown mode?

There's nothing obvious, but a possible hack would be:

_is_shutdown = False

def _on_shutdown():
    global _is_shutdown
    _is_shutdown = True

atexit.register(_on_shutdown)
msg209416 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-27 08:42
Yeah, I thought about it. Only flag should be false in shutdown mode, because it will be None when a module will be cleaned.
msg209422 - (view) Author: Roundup Robot (python-dev) Date: 2014-01-27 09:33
New changeset 50aa9e3ab9a4 by Serhiy Storchaka in branch '3.3':
Issue #19077: tempfile.TemporaryDirectory cleanup is now most likely
http://hg.python.org/cpython/rev/50aa9e3ab9a4

New changeset d792eb3afa58 by Serhiy Storchaka in branch 'default':
Issue #19077: tempfile.TemporaryDirectory cleanup no longer fails when
http://hg.python.org/cpython/rev/d792eb3afa58
msg209425 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-27 09:52
Thank you for your patch Antoine. And thank you for your excellent idea Richard.
History
Date User Action Args
2014-01-27 09:52:06serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg209425

stage: patch review -> resolved
2014-01-27 09:33:37python-devsetnosy: + python-dev
messages: + msg209422
2014-01-27 08:42:44serhiy.storchakasetmessages: + msg209416
2014-01-26 22:53:34pitrousetmessages: + msg209363
2014-01-24 17:58:10serhiy.storchakasetfiles: + tempdir_cleanup-3.3.patch, tempdir_finalize-3.4.patch

messages: + msg209105
2014-01-10 10:21:35serhiy.storchakasetfiles: + tempfile_tempdir_cleanup_2.patch

messages: + msg207842
2014-01-09 23:01:02pitrousetfiles: + tempdir_finalize.patch

messages: + msg207804
2014-01-09 22:56:14pitrousetnosy: + pitrou
messages: + msg207803
2014-01-09 22:19:41serhiy.storchakasetfiles: + tempfile_tempdir_cleanup_weakref_finalize.patch

messages: + msg207801
2014-01-09 22:04:26serhiy.storchakasetmessages: + msg207799
2014-01-09 21:54:50serhiy.storchakasetfiles: + tempfile_tempdir_cleanup.patch

messages: + msg207798
2014-01-09 21:52:52serhiy.storchakasetfiles: - tempfile_tempdir_cleanup.patch
2013-09-25 21:06:15ncoghlansetmessages: + msg198422
2013-09-25 20:07:19sbtsetnosy: + sbt
messages: + msg198414
2013-09-24 21:51:37hayposetnosy: + haypo
2013-09-23 14:58:46serhiy.storchakacreate