tempfile doesn't seem to play nicely with os.chdir on Windows

[P403n1x87]

BPO	42796
Nosy	@tiran, @eryksun, @P403n1x87
Files	tempfile_st.txt: Stacktrace

^{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 = None
closed_at = None
created_at = <Date 2020-12-31.15:44:22.972>
labels = ['library', '3.9', 'type-crash']
title = "tempfile doesn't seem to play nicely with os.chdir on Windows"
updated_at = <Date 2021-01-01.14:27:04.189>
user = 'https://github.com/P403n1x87'

bugs.python.org fields:

activity = <Date 2021-01-01.14:27:04.189>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-12-31.15:44:22.972>
creator = 'Gabriele Tornetta'
dependencies = []
files = ['49712']
hgrepos = []
issue_num = 42796
keywords = []
message_count = 5.0
messages = ['384125', '384165', '384166', '384168', '384172']
nosy_count = 3.0
nosy_names = ['christian.heimes', 'eryksun', 'Gabriele Tornetta']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue42796'
versions = ['Python 3.9']

[P403n1x87]

The following script causes havoc on Windows while it works as expected on Linux

import os
import tempfile


```py
def test_chdir():
    with tempfile.TemporaryDirectory() as tempdir:
        os.chdir(tempdir)

Running the above on Windows results in RecursionError: maximum recursion depth exceeded while calling a Python object (see attachment for the full stacktrace).

[tiran]

The code fails because TemporaryDirectory.__exit__() is unable to remove the directory. Windows doesn't let you remove files and directories that are used by a process. On POSIX-like operating systems like Linux support removing of opened files. For example anonymous temporary files use the trick.

[P403n1x87]

That makes sense, but I wonder what the "right" behaviour should be in this case. Surely the infinite recursion should be fixed at the very minimum. Perhaps the code on Windows could be enhanced to catch the case whereby one is trying to delete the cwd and do something like chdir('..') and then delete the temp folder. However, I suspect that something like this still wouldn't be enough. For example, this works fine

def test_chdir():
    with tempfile.TemporaryDirectory() as tempdir:
        old = os.getcwd()
        os.chdir(tempdir)
        os.chdir(old)
\~\~\~

whereas this doesn't (same stacktrace as the original case)

~~~ python
def test_chdir():
    with tempfile.TemporaryDirectory() as tempdir:
        old = os.getcwd()
        os.chdir(tempdir)
        with open(os.path.join(tempdir, "delme")) as fout:
            fout.write("Hello")
        os.chdir(old)
\~\~\~

[eryksun]

Windows doesn't let you remove files and directories that are used
by a process.

Windows does allow deleting open files/directories, but read/execute, write/append, and delete/rename access have to be explicitly shared when opening a file or directory. WinAPI SetCurrentDirectoryW (called by os.chdir) does not share delete access when opening a directory as the new working directory, and the handle is kept open to use as the NTAPI RootDirectory handle when opening relative paths. So the directory can only be deleted after either the working directory changes or the process exits.

See bpo-35144 for the source of the recursion error. An onerror handler was added that retries rmtree() after attempting to reset permissions. I proposed a workaround in the linked issue, but I don't think it's enough. Maybe there should be a separate onerror function for Windows.

The two common PermissionError cases to handle in Windows are readonly files and sharing violations. If the readonly attribute is set, we can remove it and retry the unlink() or rmdir() call. I don't think there's no need for a recursive call since rmtree() only removes empty directories.

For a sharing violation (winerror 32), all that can be done is to set an atexit function that retries deleting the directory. If rmtree() still fails, emit a resource warning and give up.

[eryksun]

def test_chdir():
with tempfile.TemporaryDirectory() as tempdir:
old = os.getcwd()
os.chdir(tempdir)
with open(os.path.join(tempdir, "delme")) as fout:
fout.write("Hello")
os.chdir(old)

The open() call in test_chdir() fails before os.chdir(old) executes. You would need to call os.chdir(old) in a finally block.

But the cleanup routine could still fail due to a sharing violation from some other process (likely a child process) setting the directory or a child directory as the working directory. Also, a sharing violation when trying to delete a regular file in the tree causes the cleanup routine to fail with NotADirectoryError because the onerror function calls rmtree() if unlink() fails with a PermissionError.