-
Notifications
You must be signed in to change notification settings - Fork 48
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
Recursively delete directories on Windows without race conditions #110
Comments
This is described on slide 11 of the talk. |
That makes very little sense. Slide 19 indicates that you can't rename the file if any process has a read lock. Which is exactly the problem. The rename method only allows you to prevent new processes to acquire read locks on the file if they are trying to open the file by name since renames are atomic vs delete files which is just marking for deletion. It explicitly says you still have to retry. It also doesn't take into account services that lock any new file like an AV. After you rename that file can still get locked. So I don't understand how that method "solves" it. |
Sorry, I think I may have conflated two issues:
|
I'd say this algorithm makes it less atomic and makes things more complicated in case of partial failure and exceptions. You'll end up with stray directories, no? Retry is a viable solution for both problems afais. |
The inclusion of retrying functions in |
I personally don't understand why it's being added to base and not a uncoupled library like directory. Adding it to directory makes more sense since it can then be used with any version of the compiler. Adding it to base restricts it to the bleeding edge for no reason. If this is really so contentious I'll just add it to Win32 and move on. But i don't want to split functionality across libraries unless needing to. I think we're over engineering this and over thinking a simple solution. But that's just my two cents. |
It appears that this would be the least disruptive solution. This is a Windows-specific problem, and as discussed in the GHC tracker, retrying is kind of problem-specific. |
It looks like you can use https://docs.rs/crate/remove_dir_all/0.5.3/source/src/fs.rs On another note, is there any reason The only counterargument I've seen is that the API does not work on handles rust-lang/rust#29497 (comment), which does not apply to |
This means if the programmer tries so open a new file with the same name/path they will get an access denied on
It is not entirely clear to me that these shell API functions support NT namespace paths. https://docs.microsoft.com/en-us/windows/win32/api/shellapi/ns-shellapi-shfileopstructw suspiciously mentions the buffer must be large enough to hold It is also not entirely clear that they actually solve the problem. The function has a documented return code for when a sharing violation occurred during the operation. In fact the recursive delete that it provides can fail for some files so it can return partial failures. Now you just won't know which files failed. |
To clarify, the scope of the current issue is:
To maintain current behavior, support for long paths would be a requirement. On the other hand, the following are out of scope of this issue:
|
https://bugs.python.org/issue40143#msg365697 noted that Given that, I'm curious whether Microsoft plans to (eventually) fix the issue on their end though. If that's their plan it may be better to stick to the conventional Win32 APIs and wait for them to fix it than to create our own unique workarounds. |
Niall Douglas' "Racing the File System" at CppCon actually outlines a deletion algorithm that works correctly on Windows. It does not require retries.
The gist of it:
This provides an alternative solution to the issue described in #96 and #108.
Alternative
It seems there's yet another way to do this using Windows 10 V1607's POSIX-compatible API:
But this is limited to NTFS.
https://stackoverflow.com/a/50234974
The text was updated successfully, but these errors were encountered: