Skip to content

[Windows] Atomic write: clear READONLY on destination and retry rename#2007

Open
clintonpi wants to merge 1 commit into
swiftlang:release/6.4.xfrom
clintonpi:windows-atomic-write-concurrent
Open

[Windows] Atomic write: clear READONLY on destination and retry rename#2007
clintonpi wants to merge 1 commit into
swiftlang:release/6.4.xfrom
clintonpi:windows-atomic-write-concurrent

Conversation

@clintonpi
Copy link
Copy Markdown

Motivation:

SetFileInformationByHandle with FileRenameInfoEx and FILE_RENAME_FLAG_POSIX_SEMANTICS | FILE_RENAME_FLAG_REPLACE_IF_EXISTS can fail with ERROR_ACCESS_DENIED (mapped from NTSTATUS STATUS_CANNOT_DELETE) when the destination has FILE_ATTRIBUTE_READONLY. POSIX rename(2) is not affected if the destination file itself is read-only. This is the final piece for #1485.

Fixes #1507

Modifications:

  • When the rename fails with ERROR_ACCESS_DENIED and the destination has FILE_ATTRIBUTE_READONLY set, clear that attribute and retry the rename once before falling through to the existing MoveFileExW fallback.

Result:

Atomic writes on Windows now replace a read-only destination, aligning with POSIX semantics.

Testing:

Added a cross-platform test to verify the modification and its consistency.

Motivation:

`SetFileInformationByHandle` with `FileRenameInfoEx` and `FILE_RENAME_FLAG_POSIX_SEMANTICS | FILE_RENAME_FLAG_REPLACE_IF_EXISTS` can fail with `ERROR_ACCESS_DENIED` (mapped from NTSTATUS `STATUS_CANNOT_DELETE`) when the destination has `FILE_ATTRIBUTE_READONLY`. POSIX `rename(2)` is not affected if the destination file itself is read-only. This is the final piece for swiftlang#1485.

_Fixes #1507_

Modifications:

- When the rename fails with `ERROR_ACCESS_DENIED` and the destination has `FILE_ATTRIBUTE_READONLY` set, clear that attribute and retry the rename once before falling through to the existing `MoveFileExW` fallback.

Result:

Atomic writes on Windows now replace a read-only destination, aligning with POSIX semantics.

Testing:

Added a cross-platform test to verify the modification and its consistency.
@clintonpi
Copy link
Copy Markdown
Author

@swift-ci please test

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.

2 participants