Path still exists after remove successful on windows7 SP1

[roblabla]

The Rust 1.69 update broke some fairly simple unit tests we have that runs on Windows7 SP1. Removing a file using std::fs::remove_file, and subsequently checking if it exists using Path::exists, will inexplicably return that the path still exists, instead of returning false.

Note that this behavior is not reproduced on Windows10, only on Windows7.

Code

use std::env;
use std::fs::File;
use std::path::Path;

fn main() {
    let path = env::temp_dir().join("tempfile.txt");
    let f = File::create(&path);
    std::fs::remove_file(&path).unwrap();
    assert!(!path.exists());
    println!("Path does not exist anymore");
}

I expected to see this happen: The assertion succeeds, and "Path does not exist anymore" is printed.

Instead, this happened: The assertion fails, rust believes that the path still exists.

Version it worked on

It most recently worked on Rust 1.68.2

Version with regression

rustc --version --verbose:

rustc 1.69.0 (84c898d65 2023-04-16)
binary: rustc
commit-hash: 84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc
commit-date: 2023-04-16
host: aarch64-apple-darwin
release: 1.69.0
LLVM version: 15.0.7

@rustbot modify labels: +regression-from-stable-to-stable -regression-untriaged

[ChrisDenton]

Would you be able to see what metadata() returns? Also it would be useful to know if try_exists() works as expected.

Btw, have you confirmed that 1.68.2 still works (to rule out CI changes)?

[roblabla]

Yes, I have confirmed that 1.68.2 still works.

[roblabla]

I made this little test program:

use std::env;
use std::fs::File;
use std::path::Path;

fn main() {
    let path = env::temp_dir().join("tempfile.txt");
    let f = File::create(&path);
    std::fs::remove_file(&path).unwrap();
    println!("exists = {:?}", path.exists());
    println!("metadata = {:?}", path.metadata());
    println!("try_exists = {:?}", path.try_exists());
}

1.68.0 output:

exists = false
metadata = Err(Os { code: 5, kind: PermissionDenied, message: "Access is denied." })
try_exists = Err(Os { code: 5, kind: PermissionDenied, message: "Access is denied." })

1.69.0 output:

exists = true
metadata = Ok(Metadata { file_type: FileType(FileType { attributes: 8224, reparse_tag: 0 }), is_dir: false, is_file: true, permissions: Permissions(FilePermissions { attrs: 8224 }), modified: Ok(SystemTime { intervals: 133300169448298000 }), accessed: Ok(SystemTime { intervals: 133300169448298000 }), created: Ok(SystemTime { intervals: 133300169448298000 }), .. })
try_exists = Err(Os { code: 5, kind: PermissionDenied, message: "Access is denied." })

This seems to be caused by #107985, which seems to try getting file information using FindFirstFileW instead of GetFileInformationByHandle when the later returns ERROR_ACCESS_DENIED.

I'm not sure if this is really a bug now. From fiddling with the test a bit more, I was able to determine that the cause of the issue is keeping a file handle on the file being deleted. If I std::mem::drop(f) before calling path.exists(), it returns true as expected. It looks like, when we try to delete an open file, Windows7 will remove all permissions on the file, and wait for all opened handle to be closed before fully deleting the path.

[ChrisDenton]

Ah yeah, that's how Windows 7 file deletion works. The delete only occurs when all file handles are closed.

So it seems previously checking if the file exists was causing an ACCESS_DENIED error an the exists function interpreted any and all errors as the file not existing. The bug fix made it so that e.g. permission errors may not cause metadata to fail but also made it so that it will succeed even if a delete is pending.

Windows 10 uses "POSIX semantics" under the hood so that the file is actually deleted right away.

[workingjubilee]

Per roblabla's remarks:

I'm not sure if this is really a bug now. From fiddling with the test a bit more, I was able to determine that the cause of the issue is keeping a file handle on the file being deleted. If I std::mem::drop(f) before calling path.exists(), it returns true as expected.

So I do not think this is a high priority bug in the absolute sense unless something else unexpected-yet-not-described is also a consequence of this, and it is also technically not a break of any semantic promise (we don't explicitly say "if you remove this file, then call .exists(), it will definitely return true", in fact exists comes with disclaimers about being kinda buggy), thus P-low.

In fact, if we consult fs::remove_file's docs:

Note that there is no guarantee that the file is immediately deleted (e.g., depending on platform, other open file descriptors may prevent immediate removal).

So perhaps we are obligated to close this as "wontfix"? Or perhaps there is some improvement or difference you would like to see? I will leave this open, as I am interested in what else people think.

[ChrisDenton]

After sleeping on it, I'm going to boldly close this as "working as intended" but feel free to object and I'll (or someone else will) reopen. The bug here was actually the old versions of Rust that reported the file didn't exist when it absolutely still did. According to the previous Windows implementation of exists, if a file cannot be opened for any reason then it must not exist. I personally think this is somewhat flawed reasoning. And I'm not really convinced it's worth preserving such bugs, especially once a fix is already stable, and especially especially as exists is effectively documented as a wrapper around metadata ("If you cannot access the metadata of the file..."). It would be terrible if we couldn't fix bugs with metadata in order to protect exists from becoming slightly less buggy.

Of course the other view is that the behaviour of remove_file should be changed. But how? The delete operation was performed successfully, even if the file is merely placed in a "delete pending" state until all file handles are closed. Should we wait until the file is actually deleted? Practicality of doing that aside, I think people would be pretty upset if remove_file were to cause a deadlock in common cases (deleting a file when the application has an already opened file handle). And, as jubilee says, in any case this is documented as possible platform behaviour.

In short, this is just how Windows 7 works. In a way the unit test was always broken but the error was invisible because stdlib was lying to you.