| msg119505 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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: Alyssa Coghlan (ncoghlan) * |
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) * |
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) * |
Date: 2010-12-11 19:10 |
| Applied the fix from in r87172. |
|
|
| msg123821 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
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: Alyssa Coghlan (ncoghlan) * |
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: Alyssa Coghlan (ncoghlan) * |
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) * |
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) * |
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: Alyssa Coghlan (ncoghlan) * |
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) * |
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: Alyssa Coghlan (ncoghlan) * |
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. |
|
|