classification
Title: tempfile.TemporaryDirectory may throw errors at shutdown
Type: behavior Stage:
Components: Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Jurko.Gospodnetić, eric.araujo, georg.brandl, ncoghlan, pitrou, r.david.murray, terry.reedy
Priority: normal Keywords: patch

Created on 2010-10-24 10:52 by ncoghlan, last changed 2010-12-13 16:36 by ncoghlan. This issue is now closed.

Files
File name Uploaded Description Edit
issue10888_tempdir_cleanup.diff ncoghlan, 2010-12-12 02:22 More meaningful error on stderr at shutdown, emit resource warning
Messages (14)
msg119505 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-10-24 10:52
During interpreter shutdown, modules can become unusable as module globals are set to None. This is a problem for tempfile.TemporaryDirectory, as it needs working os, os.path and stat modules in order to clean up the filesystem.

The class makes a valiant attempt at reducing the frequency of these errors, but it is ultimately useless, since the three modules internally rely on their various globals remaining valid.

Issue #812369 may be a possible solution to this problem, or perhaps even an explicit list of essential modules that are nulled out only after all other modules have been destroyed.
msg119507 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-10-24 11:14
Cleanup of sys and __builtin__ is already delayed - this particular issue could be fixed by delaying cleanup of a few more modules, along with the proposed change to GC invocation in issue #1545463.
msg119937 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-10-29 21:02
Shutdown has been a 'forever' problem ;-). It would seem sensible to me to have a fixed end-of-shutdown sequence for essential modules.
msg123807 - (view) Author: Jurko Gospodnetić (Jurko.Gospodnetić) * Date: 2010-12-11 17:12
Also this class, because it defines __del__ too simply, will display a user-unfriendly error message when cleaning up a TemporaryDirectory object whose constructor raised an exception when attempting to create its temporary folder.

  For example try to create a TemporaryDirectory with prefix="aa>aa" on Windows. That should fail as folders there can not contain '>' characters and later on in the program you should get an error message something like this one:

Exception AttributeError: "'TemporaryDirectory' object has no attribute '_closed'" in <bound method TemporaryDirectory.cleanup of <tempfile.TemporaryDirectory object at 0x00CE1E10>> ignored

  Hope this helps.

[Sorry, did not know whether to add this as a separate issue as it seemed kind of related to this one.]
msg123808 - (view) Author: Jurko Gospodnetić (Jurko.Gospodnetić) * Date: 2010-12-11 17:19
Clicked send too soon on the previous comment. :-(

  The simplest way I see you can fix the __del__ issue is to patch TemporaryDirectory.__init__() as follows:

    def __init__(self, suffix="", prefix=template, dir=None):
        self._closed = True
        self.name = mkdtemp(suffix, prefix, dir)
        self._closed = False

  This is based on the tempfile.py from the 3.2 beta 1 release on Windows.
msg123812 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-12-11 19:10
Applied the fix from msg123808 in r87172.
msg123821 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-12-12 02:22
I have a slightly better fix for that coming soon. There's a subtle race condition in the proposed approach that can lead to the temporary dir not being deleted if an asynchronous exception, such as KeyboardInterrupt, arrives after mkdtemp completes successfully, but before _closed is set back to False.

Instead, the new init code will look like:

        self._closed = False
        self.name = None # Handle mkdtemp throwing an exception
        self.name = mkdtemp(suffix, prefix, dir)

And the cleanup condition will be gated on self.name being set as well as on _closed being False. I believe there is still a window where mkdtemp successfully creates the directory, but self.name never gets set, but we want to make that window as small as possible.

I also realised this should be emitting a ResourceWarning from __del__, as well catching the errors and shut down and turning them into something more meaningful on sys.stderr.

The attached patch won't quite apply cleanly, since it predates r87172
msg123822 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-12-12 02:23
Although it may be better to just raise a new, more meaningul, exception and let the runtime take care of ignoring it (since it happens in __del__)
msg123838 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-12-12 15:26
Tidy ups committed in r87182. Shutdown problems now result in a slightly more meaningful message on stderr, a ResourceWarning is triggered when an implicit teardown occurs and the chances of an AttributeError due to an exception in __init__ are minimised.
msg123839 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-12 16:48
The tests are failing on windows:

http://www.python.org/dev/buildbot/all/builders/x86%20XP-4%203.x/builds/3770/steps/test/logs/stdio

======================================================================
ERROR: test_mkdtemp_failure (test.test_tempfile.test_TemporaryDirectory)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows\build\lib\test\test_tempfile.py", line 933, in test_mkdtemp_failure
    tempfile.TemporaryDirectory(prefix="[]<>?*!:")
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows\build\lib\tempfile.py", line 624, in __init__
    self.name = mkdtemp(suffix, prefix, dir)
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows\build\lib\tempfile.py", line 301, in mkdtemp
    _os.mkdir(file, 0o700)
WindowsError: [Error 123] The filename, directory name, or volume label syntax is incorrect: 'c:\\docume~1\\db3l\\locals~1\\temp\\[]<>?*!:9ohp4a'

======================================================================
FAIL: test_warnings_on_cleanup (test.test_tempfile.test_TemporaryDirectory)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows\build\lib\test\test_tempfile.py", line 1001, in test_warnings_on_cleanup
    self.assertIn(d.name,  message)
AssertionError: 'c:\\docume~1\\db3l\\locals~1\\temp\\94ifn5\\ujobx5' not found in 'ERROR: TypeError("\'NoneType\' object is not callable",) while cleaning up <TemporaryDirectory \'c:\\\\docume~1\\\\db3l\\\\locals~1\\\\temp\\\\94ifn5\\\\ujobx5\'>\n'
msg123840 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-12-12 16:49
There's also a typo in the issue number in your commit message (10888 instead of 10188).
msg123860 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-12-13 03:13
Sorry, I realised after I had logged off and gone to bed that I hadn't finished the last test. Fixed in r87204 with an approach that should exercise the relevant behaviour regardless of platform.

The commit message has also been updated to refer to the correct issue.

I'm actually going to close this now - the problem of misbehaviour due to modules being nulled out at shutdown is a more universal problem being tracked elsewhere, and I think the behaviour I just added is the best we can hope for until that is fixed.
msg123880 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-12-13 15:22
A test still fails under Windows:

======================================================================
FAIL: test_warnings_on_cleanup (test.test_tempfile.test_TemporaryDirectory)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows\build\lib\test\test_tempfile.py", line 1003, in test_warnings_on_cleanup
    self.assertIn(d.name,  message)
AssertionError: 'c:\\docume~1\\db3l\\locals~1\\temp\\c2h__k\\1kxw82' not found in 'ERROR: TypeError("\'NoneType\' object is not callable",) while cleaning up <TemporaryDirectory \'c:\\\\docume~1\\\\db3l\\\\locals~1\\\\temp\\\\c2h__k\\\\1kxw82\'>\n'

----------------------------------------------------------------------
msg123882 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-12-13 16:36
Ah, yes, I failed to account for the additional string escaping performed by the IO stream. I think I've been bitten by that before, but I always forget since I try to always use forward slashes for paths, even on Windows.

r87212 modifies the test to ensure that failure mode is encountered on non-Windows systems as well and correctly reverses the escaping in the test when reading the message back.
History
Date User Action Args
2010-12-13 16:36:47ncoghlansetmessages: + msg123882
2010-12-13 15:22:08pitrousetnosy: + pitrou
messages: + msg123880
2010-12-13 03:13:05ncoghlansetstatus: open -> closed
type: behavior
resolution: fixed
messages: + msg123860
2010-12-12 16:49:38r.david.murraysetmessages: + msg123840
2010-12-12 16:48:47r.david.murraysetnosy: + r.david.murray
messages: + msg123839
2010-12-12 15:26:48ncoghlansetmessages: + msg123838
2010-12-12 02:23:13ncoghlansetmessages: + msg123822
2010-12-12 02:22:13ncoghlansetfiles: + issue10888_tempdir_cleanup.diff
keywords: + patch
messages: + msg123821
2010-12-11 19:10:49georg.brandlsetnosy: + georg.brandl
messages: + msg123812
2010-12-11 17:19:46Jurko.Gospodnetićsetmessages: + msg123808
2010-12-11 17:12:47Jurko.Gospodnetićsetnosy: + Jurko.Gospodnetić
messages: + msg123807
2010-10-29 21:02:24terry.reedysetnosy: + terry.reedy
messages: + msg119937
2010-10-24 17:35:15eric.araujosetnosy: + eric.araujo

versions: + Python 3.2
2010-10-24 11:14:59ncoghlansetmessages: + msg119507
2010-10-24 10:52:59ncoghlancreate