Skip to content
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

Describe more platform-specific behaviors of std::fs::rename #31963

Merged
merged 1 commit into from
Apr 12, 2016

Conversation

barosl
Copy link
Contributor

@barosl barosl commented Feb 29, 2016

I did some tests myself regarding the situation when both from and to exist, and the results were:

On Linux:

from to Result
Directory Directory Ok
Directory File Error
File Directory Error
File File Ok

On Windows:

from to Result
Directory Directory Error
Directory File Ok
File Directory Error
File File Ok

This is a bit against the official MSDN documentation, which says "(MOVEFILE_REPLACE_EXISTING) cannot be used if lpNewFileName or lpExistingFileName names a directory." As evidenced above, lpExistingFileName can be a directory.

I also mentioned the atomicity of the operation.

Fixes #31301.

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@retep998
Copy link
Member

While MSDN doesn't explicitly say it is atomic, it is effectively atomic on Windows since we don't pass MOVEFILE_COPY_ALLOWED.

@barosl
Copy link
Contributor Author

barosl commented Feb 29, 2016

@retep998 I couldn't be sure if the operation of replacing the old, existing file can also be considered "effectively atomic". I mean, wouldn't there be a possibility that MOVEFILE_REPLACE_EXISTING is internally implemented as "if lpNewFileName already points to a file, remove it; then continue renaming"? Are you certain that the case is also OK? I've searched the web but some people believe it is atomic, but some others claim it isn't...

There's also an API named MoveFileTransacted, but strangely MS wants to make it obsolete...

@retep998
Copy link
Member

It is atomic in the sense that at point A the file is not at the destination, at point B the file is at at the destination, and there is no point C in between those two points where you could possibly see something else, at least for NTFS anyway. Not sure about FAT.

@barosl
Copy link
Contributor Author

barosl commented Feb 29, 2016

@retep998 What I wondered is if there are two files named a.txt and b.txt, and one tries to rename a.txt into b.txt, is there a point when b.txt is deleted first but a.txt is not yet renamed? If the system hangs at this point, even though the renaming isn't completed, b.txt is gone for nothing. I think if that is the case, it may not be considered atomic. But I don't know if that is the case. Does NTFS also guarantee this?

@retep998
Copy link
Member

It should also be atomic in that sense, at least when running normally. I'm not sure if it is atomic with respect to system crashes and power failures, ideally Windows would take advantage of the change journal to rollback those incidents on next boot, but I just don't know. We'd need someone from Microsoft to confirm that.

@steveklabnik
Copy link
Member

/cc @rust-lang/libs, are you happy with these docs?

@alexcrichton
Copy link
Member

Seems fine to me, yes

@barosl
Copy link
Contributor Author

barosl commented Apr 12, 2016

I removed the mention of atomicity ("Also, the operation is guaranteed to be atomic on Unix. There's currently no such guarantee on Windows."). I guess it would be better to make it unspecified, until more concrete information is found.

r? @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+ bcbc9e5

Thanks!

@bors
Copy link
Contributor

bors commented Apr 12, 2016

⌛ Testing commit bcbc9e5 with merge a4f781e...

bors added a commit that referenced this pull request Apr 12, 2016
Describe more platform-specific behaviors of `std::fs::rename`

I did some tests myself regarding the situation when both `from` and `to` exist, and the results were:

On Linux:

`from` | `to` | Result
---- | ---- | ----
Directory | Directory | Ok
Directory | File | Error
File | Directory | Error
File | File | Ok

On Windows:

`from` | `to` | Result
---- | ---- | ----
Directory | Directory | Error
Directory | File | Ok
File | Directory | Error
File | File | Ok

This is a bit against the official MSDN documentation, which says "(`MOVEFILE_REPLACE_EXISTING`) cannot be used if `lpNewFileName` or `lpExistingFileName` names a directory." As evidenced above, `lpExistingFileName` *can* be a directory.

I also mentioned the atomicity of the operation.

Fixes #31301.
@bors bors merged commit bcbc9e5 into rust-lang:master Apr 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants