Skip to content

os.unlink behaviour is error prone on windows #109608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
inorton opened this issue Sep 20, 2023 · 6 comments
Open

os.unlink behaviour is error prone on windows #109608

inorton opened this issue Sep 20, 2023 · 6 comments
Labels
OS-windows type-feature A feature request or enhancement

Comments

@inorton
Copy link
Contributor

inorton commented Sep 20, 2023

Bug report

Bug description:

There are a great many bugs, mostly relating to use of shutil.rmtree(), os.unlink() or os.remove() on windows where people find they have to retry errors when deleting or renaming files, or when creating fresh files with the same name of files they have deleted using these calls.

To pick one at random #51692 outlines the classic issue and also notes the cause.

On windows DeleteFileW() will return success when a file has been "schedulled" for deletion. Since almost everyone who writes code that calls os.unlink() expects the file to be gone once that function returns this is the cause of many windows-specific errors in code that otherwise works fine on all other platforms.

The Windows API docs for DeleteFile() at https://learn.microsoft.com/en-gb/windows/win32/api/fileapi/nf-fileapi-deletefilew?redirectedfrom=MSDN state:

The DeleteFile function marks a file for deletion on close. Therefore, the file deletion does not occur until the last handle to the file is closed

Many comments out there wrongly attribute the resulting errors to antivirus scanners delaying the removal of the file, but to me it seems like cpython should, on windows handle this, it is simply a longer delay on windows if antivirus is busy with a file we wanted to remove.

It therefore seems reasonable to consider making os.unlink() / os.remove() on windows block until the file has actually been removed if DeleteFileW() has not returned an error.

Has this been considered before? In my admittedly quick search of the current and past issues I've not seen this proposed.

Many thanks.

Ian

CPython versions tested on:

3.8, 3.9, 3.10, 3.11, 3.12, CPython main branch

Operating systems tested on:

Windows

@inorton inorton added the type-bug An unexpected behavior, bug, or error label Sep 20, 2023
@zooba
Copy link
Member

zooba commented Sep 20, 2023

Someone could contribute an improvement to shutil.rmtree to make it work, but adding delays to os.unlink would be devastating for anyone unlinking multiple files.

The problem doesn't arise until you do something to reuse the filename. We shouldn't assume that someone is only deleting a file so that they can recreate it.

It therefore seems reasonable to consider making os.unlink() / os.remove() on windows block until the file has actually been removed

If the app itself has the file open, blocking until all handles are closed would deadlock the app. This would be as bad (or worse) as the "safely remove disk" command.

@zooba zooba added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Sep 20, 2023
@inorton
Copy link
Contributor Author

inorton commented Sep 20, 2023

@zooba I don't 100% agree about the "devastating" part for speed, but I do accept it would take longer, simply because right now it doesn't wait for the file to actually go away.

As for waiting forever because the file was open, yes, I can see that being a change in behaviour, however, if the file was open by anything other than a scanner opening the file with FILE_SHARE_DELETE share mode, then the call to DeleteFile would fail rather than return success. So if our own process or some other regular process had the file open, the os.unlink() call would raise an exception rather than enter the wait period.

import os
open("file.txt", "w").close()
f = open("file.txt", "r")
os.unlink("file.txt")

raises PermissionError: Win32Error because the file is in use.

(On linux etc this wouldn't error)

@inorton
Copy link
Contributor Author

inorton commented Sep 20, 2023

Perhaps it might be better for shutil.rmtree to rename the folder before deletion

@csm10495
Copy link
Contributor

I've hit this issue with unlink on Windows before. Can we at least update the docs to note why Windows may seem different?

(Add a note that unlink schedules the delete on Windows but doesn't wait for it to be done).

@inorton
Copy link
Contributor Author

inorton commented Sep 24, 2023

@csm10495 That sounds a good idea. Also, updating shutil to DWIM seems less disruptive than altering unlink

@inorton
Copy link
Contributor Author

inorton commented Sep 26, 2023

Found another earlier issue https://bugs.python.org/issue22024

I'm currently working on a patch to shutil that hopefully covers these cases in a friendly way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-windows type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants