Repository navigation
tempfile.TemporaryDirectory fails to delete itself #74168
Description
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 2021-03-14.18:11:24.171>
created_at = <Date 2017-04-04.18:26:18.456>
labels = ['3.7', 'type-bug', 'library', 'OS-windows']
title = 'tempfile.TemporaryDirectory fails to delete itself'
updated_at = <Date 2021-03-14.18:11:24.171>
user = 'https://github.com/pkch'bugs.python.org fields:
activity = <Date 2021-03-14.18:11:24.171>
actor = 'gvanrossum'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2021-03-14.18:11:24.171>
closer = 'gvanrossum'
components = ['Library (Lib)', 'Windows']
creation = <Date 2017-04-04.18:26:18.456>
creator = 'max'
dependencies = []
files = []
hgrepos = []
issue_num = 29982
keywords = ['patch']
message_count = 6.0
messages = ['291130', '292562', '388219', '388246', '388318', '388680']
nosy_count = 9.0
nosy_names = ['gvanrossum', 'paul.moore', 'tim.golden', 'max', 'zach.ware', 'serhiy.storchaka', 'steve.dower', 'CAM-Gerlach', 'Jeffrey.Kintscher']
pr_nums = ['24793']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29982'
versions = ['Python 3.6', 'Python 3.7']
Activity
pkch commentedon Apr 4, 2017
There's a known issue with
shutil.rmtreeon Windows, in that it fails intermittently.The issue is well known (https://mail.python.org/pipermail/python-dev/2013-September/128353.html), and the agreement is that it cannot be cleanly solved inside
shutiland should instead be solved by the calling app. Specifically, python devs themselves faced it in their test suite and solved it by retrying delete.However, what to do about
tempfile.TemporaryDirectory? Is it considered the calling app, and therefore should retry delete when it callsshutil.rmtreein itscleanupmethod?I don't think
tempfileis protected by the same argument thatshutil.rmtreeis protected, in that it's too messy to solve it in the standard library. My rationale is that while it's very easy for the end user to retryshutil.rmtree, it's far more difficult to fix the problem withtempfile.TempDirectorynot deleting itself - how would the end user retry thecleanupmethod (which is called fromweakref.finalizer)?So perhaps the retry loop should be added to
cleanup.gvanrossum commentedon Apr 29, 2017
A simpler approach would be to simply ignore the error when it occurs in TemporaryDirectory._cleanup. There are no absolute guarantees about tempfile always cleaning up -- just a best effort. The complexity (and potential delays) of a retry loop seem more risky than simply occasionally not cleaning up -- that happens anyways, for a variety of reasons.
CAM-Gerlach commentedon Mar 7, 2021
In addition to transient failures, this can also occur when, for example, files opened in the temporary directory (perhaps by library or application code not under direct control of the caller) haven't been properly cleaned up and their file handles don't get closed, resulting in permissions errors trying to delete them (particularly on platforms like Windows, that automatically lock files when opening them).
This case came up in e.g. this recent PR and rendered
tempfile.TemporaryDirectoryunusable for such use cases, forcing a reversion to the lower-leveltempfile.mkdtempwithout the cleaner, more robust and easier to interpret high-level interface that the former provides. Wrapping awithstatement in a try-finally is syntactically ugly and semantically incongruous, and requires a second shutil.rmtree(..., ignore_errors=True)call to clean up in a best-effort manner, while when manually callingcleanup()` in a try-except, the finalizer still gets executed at a a non-deterministic later time when Python exits, raising an error.Therefore, in the spirit of Guido's statements above in terms of providing a "best-effort" cleanup, I propose (and am willing to submit a PR to implement) adding an
ignore_errorsbool parameter (defaulting to False, of course, for backward compat--and should it be keyword only likeerrorstoTemporaryFile?) to thetempfile.TemporaryDirectoryconstructor, which gets passed toshutil.rmtreeon cleanup. This would not only address both cases here, but also one of the two discussed by Anthony Sotitle on BPO-25024, in a cleaner and simpler fashion that would take advantage of existingtempfile.TemporaryDirectoryfunctionality and behavior.Would a PR be accepted on this? If so, any specific guidance on tests and whether to mention it in What's New, etc., would be appreciated. Thanks!
gvanrossum commentedon Mar 7, 2021
SGTM
CAM-Gerlach commentedon Mar 8, 2021
Thanks! Submitted as PR bpo-24793 #24793
gvanrossum commentedon Mar 14, 2021
New changeset bd2fa3c by CAM Gerlach in branch 'master':
bpo-29982: Add "ignore_cleanup_errors" param to tempfile.TemporaryDirectory() (GH-24793)
bd2fa3c
Ignore TempDir clean up error.
Ignore TempDir clean up error.
Ignore TempDir clean up error (#2379)
FIX: Fix PermissionError on Win (#105)