TemporaryDirectory attempts to clean up twice

[oconnor663]

BPO	22427
Nosy	@pitrou, @vstinner, @jwilk, @serhiy-storchaka, @MojoVampire, @oconnor663
Files	tempfile_exit_on_shutdown.patch tempfile_exit_on_shutdown2.patch tempfile_exit_on_shutdown3.patch

^{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:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2014-09-24.10:37:25.221>
created_at = <Date 2014-09-17.05:18:39.784>
labels = ['type-bug', 'library']
title = 'TemporaryDirectory attempts to clean up twice'
updated_at = <Date 2014-09-24.14:43:42.652>
user = 'https://github.com/oconnor663'

bugs.python.org fields:

activity = <Date 2014-09-24.14:43:42.652>
actor = 'vstinner'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2014-09-24.10:37:25.221>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2014-09-17.05:18:39.784>
creator = 'oconnor663'
dependencies = []
files = ['36644', '36645', '36658']
hgrepos = []
issue_num = 22427
keywords = ['patch']
message_count = 12.0
messages = ['226979', '226996', '227006', '227012', '227013', '227085', '227088', '227089', '227099', '227435', '227438', '227456']
nosy_count = 7.0
nosy_names = ['pitrou', 'vstinner', 'jwilk', 'python-dev', 'serhiy.storchaka', 'josh.r', 'oconnor663']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22427'
versions = ['Python 3.4', 'Python 3.5']

[oconnor663]

The following little script prints (but ignores) a FileNotFoundError.

import tempfile
def generator():
    with tempfile.TemporaryDirectory():
        yield
g = generator()
next(g)

Exception ignored in: <generator object generator at 0x7fb319fe2c60>
Traceback (most recent call last):
  File "gen.py", line 6, in generator
  File "/usr/lib/python3.4/tempfile.py", line 691, in __exit__
  File "/usr/lib/python3.4/tempfile.py", line 697, in cleanup
  File "/usr/lib/python3.4/shutil.py", line 454, in rmtree
  File "/usr/lib/python3.4/shutil.py", line 452, in rmtree
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmp7wek4xhy'

Putting print statements in the TemporaryDirectory class shows what's happening (confirming Guido's theory from https://groups.google.com/forum/#!topic/python-tulip/QXgWH32P2uM): As the program exits, the TemporaryDirectory object is finalized. This actually rmtree's the directory. After that, the generator is finalized, which raises a GeneratorExit inside of it. That exception causes the with statement to call __exit__ on the already-finalized TemporaryDirectory, which tries to rmtree again and throws the FileNotFoundError.

The main downside of this bug is just garbage on stderr. I suppose in exceptionally unlikely circumstances, a new temp dir by the same name could be created between the two calls, and we might actually delete something we shouldn't. The simple fix would be to store a _was_cleaned flag or something on the object. Is there any black magic in finalizers or multithreading that demands something more complicated?

[vstinner]

On Python 3.5 compiled in debug mode (or probably with python -Wd when compiled in release mode), I got this warning:

/home/haypo/prog/python/default/Lib/tempfile.py:708: ResourceWarning: Implicitly cleaning up <TemporaryDirectory '/tmp/tmpcr8u3m8v'>
_warnings.warn(warn_message, ResourceWarning)

The script is really a corner case: the generator is garbage collected whereas it is not done. At the same time, the TemporaryDirectory object is also garbage collected. I guess that the exact order of object deletion is not reliable: the generator may be deleted before or after the TemporaryDirectory.

TemporaryDirectory._cleanup() is called by the finalizer (_weakref.finalize) of the TemporaryDirectory, which means that the TemporaryDirectory object is garbage collected.

TemporaryDirectory.cleanup() is called indirectly by
TemporaryDirectory.__exit__().

I'm suprised that deleting a generator calls __exit__().

The following script has a more reliable behaviour: TemporaryDirectory.__exit__() is called when the generator is deleted, and TemporaryDirectory.cleanup() is not called.
---

import tempfile
import gc

def generator():
    with tempfile.TemporaryDirectory():
        print("before yield")
        yield
        print("after yield")
g = generator()
next(g)

g = None
gc.collect()

---

[oconnor663]

My example script is definitely a corner case, but where this actually came up for me was in asyncio. Using a TemporaryDirectory inside a coroutine creates a similar situation.

[serhiy-storchaka]

Here is a patch which may fix this issue.

[serhiy-storchaka]

Here is alternative patch. It simplifies TemporaryDirectory instead of complicate it.

[serhiy-storchaka]

Fixed a bug in the test and partially addressed Victor's and Yury's comments.

Antoine, could your pleas answer following question?

Is it safe to remove the "self._finalizer is not None" check in cleanup()? I.e. is it possible that self._finalizer becomes None at garbage collection?

[pitrou]

self._finalizer can be None if an exception was raised during __init__().

[serhiy-storchaka]

But in this case cleanup() can't be called.

[pitrou]

Ah... you are right. It seems the None test has been superfluous all the time.

[python-dev]

New changeset 7ea2153eae87 by Serhiy Storchaka in branch '3.4':
Issue bpo-22427: TemporaryDirectory no longer attempts to clean up twice when
https://hg.python.org/cpython/rev/7ea2153eae87

New changeset e9d4288c32de by Serhiy Storchaka in branch 'default':
Issue bpo-22427: TemporaryDirectory no longer attempts to clean up twice when
https://hg.python.org/cpython/rev/e9d4288c32de

[serhiy-storchaka]

Committed without this test.

Thank you Victor and Yury for your comments.