Skip to content

Win: Fix std::fs::rename failing on Windows Server by attempting the non-atomic rename first #138133

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.

Sign up for GitHub

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
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Win: Fix std::fs::rename failing on Windows Server by attempting the non-atomic rename first #138133

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 127 additions & 54 deletions library/std/src/sys/fs/windows.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1233,74 +1233,147 @@ pub fn unlink(path: &WCStr) -> io::Result<()> {
}

pub fn rename(old: &WCStr, new: &WCStr) -> io::Result<()> {
if unsafe { c::MoveFileExW(old.as_ptr(), new.as_ptr(), c::MOVEFILE_REPLACE_EXISTING) } == 0 {
let err = api::get_last_error();
// if `MoveFileExW` fails with ERROR_ACCESS_DENIED then try to move
// the file while ignoring the readonly attribute.
// This is accomplished by calling `SetFileInformationByHandle` with `FileRenameInfoEx`.
if err == WinError::ACCESS_DENIED {
let mut opts = OpenOptions::new();
opts.access_mode(c::DELETE);
opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT | c::FILE_FLAG_BACKUP_SEMANTICS);
let Ok(f) = File::open_native(&old, &opts) else { return Err(err).io_result() };

// Calculate the layout of the `FILE_RENAME_INFO` we pass to `SetFileInformation`
// This is a dynamically sized struct so we need to get the position of the last field to calculate the actual size.
let Ok(new_len_without_nul_in_bytes): Result<u32, _> =
((new.count_bytes() - 1) * 2).try_into()
else {
return Err(err).io_result();
// Calculate the layout of the `FILE_RENAME_INFO` we pass to `SetFileInformation`
// This is a dynamically sized struct so we need to get the position of the last field to calculate the actual size.
let Ok(new_len_without_nul_in_bytes): Result<u32, _> = ((new.count_bytes() - 1) * 2).try_into()
else {
return Err(WinError::INVALID_PARAMETER).io_result();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably ERROR_FILENAME_EXCED_RANGE would be better or else a custom error message. Even if it's not strictly what happened before.

};
let offset: u32 = offset_of!(c::FILE_RENAME_INFO, FileName).try_into().unwrap();
let struct_size = offset + new_len_without_nul_in_bytes + 2;
let layout =
Layout::from_size_align(struct_size as usize, align_of::<c::FILE_RENAME_INFO>()).unwrap();

let create_file = |extra_access, extra_flags| {
let handle = unsafe {
HandleOrInvalid::from_raw_handle(c::CreateFileW(
old.as_ptr(),
c::SYNCHRONIZE | c::DELETE | extra_access,
c::FILE_SHARE_READ | c::FILE_SHARE_WRITE | c::FILE_SHARE_DELETE,
ptr::null(),
c::OPEN_EXISTING,
c::FILE_ATTRIBUTE_NORMAL | c::FILE_FLAG_BACKUP_SEMANTICS | extra_flags,
ptr::null_mut(),
))
};

OwnedHandle::try_from(handle).map_err(|_| io::Error::last_os_error())
};

// The following code replicates `MoveFileEx`'s behavior as reverse-engineered from its disassembly.
// If `old` refers to a mount point, we move it instead of the target.
let f = match create_file(c::FILE_READ_ATTRIBUTES, c::FILE_FLAG_OPEN_REPARSE_POINT) {
Ok(handle) => {
let mut file_attribute_tag_info: MaybeUninit<c::FILE_ATTRIBUTE_TAG_INFO> =
MaybeUninit::uninit();

let result = unsafe {
cvt(c::GetFileInformationByHandleEx(
handle.as_raw_handle(),
c::FileAttributeTagInfo,
file_attribute_tag_info.as_mut_ptr().cast(),
mem::size_of::<c::FILE_ATTRIBUTE_TAG_INFO>().try_into().unwrap(),
))
};
let offset: u32 = offset_of!(c::FILE_RENAME_INFO, FileName).try_into().unwrap();
let struct_size = offset + new_len_without_nul_in_bytes + 2;
let layout =
Layout::from_size_align(struct_size as usize, align_of::<c::FILE_RENAME_INFO>())
.unwrap();

// SAFETY: We allocate enough memory for a full FILE_RENAME_INFO struct and a filename.
let file_rename_info;
unsafe {
file_rename_info = alloc(layout).cast::<c::FILE_RENAME_INFO>();
if file_rename_info.is_null() {
return Err(io::ErrorKind::OutOfMemory.into());

if let Err(err) = result {
if err.raw_os_error() == Some(c::ERROR_INVALID_PARAMETER as _)
|| err.raw_os_error() == Some(c::ERROR_INVALID_FUNCTION as _)
{
// `GetFileInformationByHandleEx` documents that not all underlying drivers support all file information classes.
// Since we know we passed the correct arguments, this means the underlying driver didn't understand our request;
// `MoveFileEx` proceeds by reopening the file without inhibiting reparse point behavior.
None
} else {
Some(Err(err))
}
} else {
// SAFETY: The struct has been initialized by GetFileInformationByHandleEx
let file_attribute_tag_info = unsafe { file_attribute_tag_info.assume_init() };
let file_type = FileType::new(
file_attribute_tag_info.FileAttributes,
file_attribute_tag_info.ReparseTag,
);

if file_type.is_symlink() {
// The file is a mount point, junction point or symlink so
// don't reopen the file so that the link gets renamed.
Some(Ok(handle))
} else {
// Otherwise reopen the file without inhibiting reparse point behavior.
None
}
}
}
// The underlying driver may not support `FILE_FLAG_OPEN_REPARSE_POINT`: Retry without it.
Err(err) if err.raw_os_error() == Some(c::ERROR_INVALID_PARAMETER as _) => None,
Err(err) => Some(Err(err)),
}
.unwrap_or_else(|| create_file(0, 0))?;
Copy link
Member

@ChrisDenton ChrisDenton Jun 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the error codes that are handled I worry this doesn't account for reparse points that cannot be opened, not because they're aren't supported but because they cannot be interpreted by CreateFile. E.g. app execution aliases aren't actually links in the filesystem sense and can only be interpreted by CreateProcess. We should still be able to rename them though.


(&raw mut (*file_rename_info).Anonymous).write(c::FILE_RENAME_INFO_0 {
Flags: c::FILE_RENAME_FLAG_REPLACE_IF_EXISTS
| c::FILE_RENAME_FLAG_POSIX_SEMANTICS,
});
// SAFETY: We allocate enough memory for a full FILE_RENAME_INFO struct and a filename.
let file_rename_info;
unsafe {
file_rename_info = alloc(layout).cast::<c::FILE_RENAME_INFO>();
if file_rename_info.is_null() {
return Err(io::ErrorKind::OutOfMemory.into());
}

(&raw mut (*file_rename_info).RootDirectory).write(ptr::null_mut());
// Don't include the NULL in the size
(&raw mut (*file_rename_info).FileNameLength).write(new_len_without_nul_in_bytes);
(&raw mut (*file_rename_info).Anonymous).write(c::FILE_RENAME_INFO_0 {
Flags: c::FILE_RENAME_FLAG_REPLACE_IF_EXISTS | c::FILE_RENAME_FLAG_POSIX_SEMANTICS,
});

new.as_ptr().copy_to_nonoverlapping(
(&raw mut (*file_rename_info).FileName).cast::<u16>(),
new.count_bytes(),
);
(&raw mut (*file_rename_info).RootDirectory).write(ptr::null_mut());
// Don't include the NULL in the size
(&raw mut (*file_rename_info).FileNameLength).write(new_len_without_nul_in_bytes);

new.as_ptr().copy_to_nonoverlapping(
(&raw mut (*file_rename_info).FileName).cast::<u16>(),
new.count_bytes(),
);
}

let result = cvt(unsafe {
c::SetFileInformationByHandle(
f.as_raw_handle(),
c::FileRenameInfoEx,
file_rename_info.cast::<c_void>(),
struct_size,
)
});

let result = match result {
Ok(i) => Ok(i),
// Windows version older than Windows 10 1607 don't support FileRenameInfoEx and fail with ERROR_INVALID_PARAMETER.
// On Windows 10 1607, it may fail with STATUS_VOLUME_NOT_UPGRADED, which gets mapped to ERROR_INVALID_FUNCTION.
// On Windows Server 2022, it may fail on ReFS with ERROR_NOT_SUPPORTED.
Err(err)
if err.raw_os_error() == Some(c::ERROR_INVALID_PARAMETER as _)
|| err.raw_os_error() == Some(c::ERROR_INVALID_FUNCTION as _)
|| err.raw_os_error() == Some(c::ERROR_NOT_SUPPORTED as _) =>
{
unsafe {
(&raw mut (*file_rename_info).Anonymous)
.write(c::FILE_RENAME_INFO_0 { ReplaceIfExists: true });
}

let result = unsafe {
cvt(unsafe {
c::SetFileInformationByHandle(
f.as_raw_handle(),
c::FileRenameInfoEx,
c::FileRenameInfo,
file_rename_info.cast::<c_void>(),
struct_size,
)
};
unsafe { dealloc(file_rename_info.cast::<u8>(), layout) };
if result == 0 {
if api::get_last_error() == WinError::DIR_NOT_EMPTY {
return Err(WinError::DIR_NOT_EMPTY).io_result();
} else {
return Err(err).io_result();
}
}
} else {
return Err(err).io_result();
})
}
Err(err) => Err(err),
};

unsafe {
dealloc(file_rename_info.cast::<u8>(), layout);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now fairly distant from the alloc. While nothing returns or panics before this atm, I wouldn't be surprised if some refactoring in the future missed this subtlety. So I would be happier with a drop guard or some other custom struct that is guaranteed to free memory wherever the scope ends.

}
Ok(())

result.map(|_| ())
}

pub fn rmdir(p: &WCStr) -> io::Result<()> {
Expand Down
Loading