Skip to content

Commit 24a6f51

Browse files
committed
FoundationEssentials: try to make the file atomic behaviour more robust
We have observed `ERROR_ACCESS_DENIED` in CI on `SetRenameInformationFile`. Try to make the path more robust by first performing a kernel based rename with POSIX semantics. This requires Windows 10 1809+. If that is unsuccessful, verify that attributes are not to blame. A failure may still occur if the file is on a different volume. In such a case, fallback to the `MoveFileExW` operation to perform a copy + delete operation. Hopefully this should make the implementation more robust to failures.
1 parent 081798c commit 24a6f51

File tree

2 files changed

+123
-26
lines changed

2 files changed

+123
-26
lines changed

Sources/FoundationEssentials/Data/Data+Writing.swift

Lines changed: 103 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ internal import DarwinPrivate // for VREG
1919

2020
internal import _FoundationCShims
2121

22+
import WinSDK
23+
2224
#if canImport(Darwin)
2325
import Darwin
2426
#elseif canImport(Android)
@@ -387,44 +389,119 @@ private func writeToFileAux(path inPath: PathOrURL, buffer: UnsafeRawBufferPoint
387389
// TODO: Somehow avoid copying back and forth to a String to hold the path
388390

389391
#if os(Windows)
390-
try inPath.path.withNTPathRepresentation { pwszPath in
391-
var (fd, auxPath, temporaryDirectoryPath) = try createProtectedTemporaryFile(at: inPath.path, inPath: inPath, options: options, variant: "Folder")
392+
var (fd, auxPath, temporaryDirectoryPath) = try createProtectedTemporaryFile(at: inPath.path, inPath: inPath, options: options, variant: "Folder")
392393

393-
// Cleanup temporary directory
394-
defer { cleanupTemporaryDirectory(at: temporaryDirectoryPath) }
394+
// Cleanup temporary directory
395+
defer { cleanupTemporaryDirectory(at: temporaryDirectoryPath) }
395396

396-
guard fd >= 0 else {
397-
throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false)
397+
guard fd >= 0 else {
398+
throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false)
399+
}
400+
401+
defer { if fd >= 0 { _close(fd) } }
402+
403+
let callback = (reportProgress && Progress.current() != nil) ? Progress(totalUnitCount: Int64(buffer.count)) : nil
404+
405+
do {
406+
try write(buffer: buffer, toFileDescriptor: fd, path: inPath, parentProgress: callback)
407+
} catch {
408+
try auxPath.withNTPathRepresentation { pwszAuxPath in
409+
_ = DeleteFileW(pwszAuxPath)
398410
}
399411

400-
defer { if fd >= 0 { _close(fd) } }
412+
if callback?.isCancelled ?? false {
413+
throw CocoaError(.userCancelled)
414+
} else {
415+
throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false)
416+
}
417+
}
401418

402-
let callback = (reportProgress && Progress.current() != nil) ? Progress(totalUnitCount: Int64(buffer.count)) : nil
419+
writeExtendedAttributes(fd: fd, attributes: attributes)
403420

404-
do {
405-
try write(buffer: buffer, toFileDescriptor: fd, path: inPath, parentProgress: callback)
406-
} catch {
407-
try auxPath.withNTPathRepresentation { pwszAuxPath in
408-
_ = DeleteFileW(pwszAuxPath)
409-
}
421+
_close(fd)
422+
fd = -1
410423

411-
if callback?.isCancelled ?? false {
412-
throw CocoaError(.userCancelled)
413-
} else {
414-
throw CocoaError.errorWithFilePath(inPath, errno: errno, reading: false)
424+
try auxPath.withNTPathRepresentation { pwszAuxiliaryPath in
425+
var hFile = CreateFileW(pwszAuxiliaryPath, DELETE,
426+
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
427+
nil, OPEN_EXISTING,
428+
FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT,
429+
nil)
430+
if hFile == INVALID_HANDLE_VALUE {
431+
let dwError = GetLastError()
432+
_ = DeleteFileW(pwszAuxiliaryPath)
433+
throw CocoaError.errorWithFilePath(inPath, win32: dwError, reading: false)
434+
}
435+
defer {
436+
switch hFile {
437+
case INVALID_HANDLE_VALUE:
438+
break
439+
default:
440+
_ = CloseHandle(hFile)
415441
}
416442
}
417443

418-
writeExtendedAttributes(fd: fd, attributes: attributes)
444+
try inPath.path.withNTPathRepresentation { pwszPath in
445+
let cbSize = wcslen(pwszPath) * MemoryLayout<WCHAR>.size
446+
let dwSize = DWORD(MemoryLayout<FILE_RENAME_INFO>.size + cbSize)
447+
try withUnsafeTemporaryAllocation(byteCount: Int(dwSize),
448+
alignment: MemoryLayout<FILE_RENAME_INFO>.alignment) { pBuffer in
449+
var pInfo = pBuffer.baseAddress?.bindMemory(to: FILE_RENAME_INFO.self, capacity: 1)
450+
pInfo?.pointee.Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS | FILE_RENAME_FLAG_REPLACE_IF_EXISTS
451+
pInfo?.pointee.RootDirectory = nil
452+
pInfo?.pointee.FileNameLength = DWORD(cbSize)
453+
let count = wcslen(pwszPath)
454+
pBuffer.baseAddress?.advanced(by: MemoryLayout<FILE_RENAME_INFO>.offset(of: \.FileName)!)
455+
.withMemoryRebound(to: WCHAR.self, capacity: count + 1) {
456+
wcscpy_s($0, count + 1, pwszPath)
457+
$0[count] = 0
458+
}
419459

420-
_close(fd)
421-
fd = -1
460+
if !SetFileInformationByHandle(hFile, FileRenameInfoEx, pInfo, dwSize) {
461+
let dwError = GetLastError()
462+
if dwError == ERROR_ACCESS_DENIED {
463+
let dwAttributes = GetFileAttributesW(pwszPath)
464+
if dwAttributes > 0,
465+
dwAttributes & (FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM) != 0 {
466+
// The target file is read-only, hidden or a system file. Remove those attributes and try again.
467+
if SetFileAttributesW(pwszPath, dwAttributes & ~(FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM)) {
468+
if SetFileInformationByHandle(hFile, FileRenameInfoEx, pInfo, dwSize) {
469+
return
470+
}
471+
// Try to restore the attributes, but ignore any error
472+
_ = SetFileAttributesW(pwszPath, dwAttributes)
473+
}
474+
}
475+
}
476+
guard dwError == ERROR_NOT_SAME_DEVICE else {
477+
_ = DeleteFileW(pwszAuxiliaryPath)
478+
throw CocoaError.errorWithFilePath(inPath, win32: dwError, reading: false)
479+
}
422480

423-
try auxPath.withNTPathRepresentation { pwszAuxiliaryPath in
424-
guard MoveFileExW(pwszAuxiliaryPath, pwszPath, MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH) else {
425-
let dwError = GetLastError()
426-
_ = DeleteFileW(pwszAuxiliaryPath)
427-
throw CocoaError.errorWithFilePath(inPath, win32: dwError, reading: false)
481+
_ = CloseHandle(hFile)
482+
hFile = INVALID_HANDLE_VALUE
483+
484+
// The move is across volumes.
485+
guard MoveFileExW(pwszAuxiliaryPath, pwszPath, MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING) else {
486+
let dwError = GetLastError()
487+
if dwError == ERROR_ACCESS_DENIED {
488+
let dwAttributes = GetFileAttributesW(pwszPath)
489+
if dwAttributes > 0,
490+
dwAttributes & (FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM) != 0 {
491+
// The target file is read-only, hidden or a system file. Remove those attributes and try again.
492+
if SetFileAttributesW(pwszPath, dwAttributes & ~(FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM)) {
493+
if MoveFileExW(pwszAuxiliaryPath, pwszPath, MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING) {
494+
return
495+
}
496+
// Try to restore the attributes, but ignore any error
497+
_ = SetFileAttributesW(pwszPath, dwAttributes)
498+
}
499+
}
500+
}
501+
_ = DeleteFileW(pwszAuxiliaryPath)
502+
throw CocoaError.errorWithFilePath(inPath, win32: dwError, reading: false)
503+
}
504+
}
428505
}
429506
}
430507
}

Sources/FoundationEssentials/WinSDK+Extensions.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ package var CREATE_NEW: DWORD {
4141
DWORD(WinSDK.CREATE_NEW)
4242
}
4343

44+
package var DELETE: DWORD {
45+
DWORD(WinSDK.DELETE)
46+
}
47+
4448
package var ERROR_ACCESS_DENIED: DWORD {
4549
DWORD(WinSDK.ERROR_ACCESS_DENIED)
4650
}
@@ -133,10 +137,18 @@ package var FILE_ATTRIBUTE_READONLY: DWORD {
133137
DWORD(WinSDK.FILE_ATTRIBUTE_READONLY)
134138
}
135139

140+
package var FILE_ATTRIBUTE_HIDDEN: DWORD {
141+
DWORD(WinSDK.FILE_ATTRIBUTE_HIDDEN)
142+
}
143+
136144
package var FILE_ATTRIBUTE_REPARSE_POINT: DWORD {
137145
DWORD(WinSDK.FILE_ATTRIBUTE_REPARSE_POINT)
138146
}
139147

148+
package var FILE_ATTRIBUTE_SYSTEM: DWORD {
149+
DWORD(WinSDK.FILE_ATTRIBUTE_SYSTEM)
150+
}
151+
140152
package var FILE_FLAG_BACKUP_SEMANTICS: DWORD {
141153
DWORD(WinSDK.FILE_FLAG_BACKUP_SEMANTICS)
142154
}
@@ -153,6 +165,14 @@ package var FILE_NAME_NORMALIZED: DWORD {
153165
DWORD(WinSDK.FILE_NAME_NORMALIZED)
154166
}
155167

168+
package var FILE_RENAME_FLAG_POSIX_SEMANTICS: DWORD {
169+
DWORD(WinSDK.FILE_RENAME_FLAG_POSIX_SEMANTICS)
170+
}
171+
172+
package var FILE_RENAME_FLAG_REPLACE_IF_EXISTS: DWORD {
173+
DWORD(WinSDK.FILE_RENAME_FLAG_REPLACE_IF_EXISTS)
174+
}
175+
156176
package var FILE_SHARE_DELETE: DWORD {
157177
DWORD(WinSDK.FILE_SHARE_DELETE)
158178
}

0 commit comments

Comments
 (0)