Skip to content

Commit f937bd1

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 f937bd1

File tree

2 files changed

+101
-27
lines changed

2 files changed

+101
-27
lines changed

Sources/FoundationEssentials/Data/Data+Writing.swift

Lines changed: 97 additions & 27 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,112 @@ 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)
415-
}
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+
if hFile == INVALID_HANDLE_VALUE { return }
437+
_ = CloseHandle(hFile)
416438
}
417439

418-
writeExtendedAttributes(fd: fd, attributes: attributes)
440+
try inPath.path.withNTPathRepresentation { pwszPath in
441+
let cbSize = wcslen(pwszPath) * MemoryLayout<WCHAR>.size
442+
let dwSize = DWORD(MemoryLayout<FILE_RENAME_INFO>.size + cbSize)
443+
withUnsafeTemporaryAllocation(byteCount: Int(dwSize),
444+
alignment: MemoryLayout<FILE_RENAME_INFO>.alignment) { pBuffer in
445+
var pInfo = pBuffer.baseAddress?.bindMemory(to: FILE_RENAME_INFO.self, capacity: 1)
446+
pInfo?.pointee.Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS | FILE_RENAME_FLAG_REPLACE_IF_EXISTS
447+
pInfo?.pointee.RootDirectory = nil
448+
pInfo?.pointee.FileNameLength = DWORD(cbSize)
449+
withUnsafePointer(to: &pInfo?.pointee.FileName) { pFileName in
450+
wcscpy_s(pFileName, wcslen(pwszPath) + 1, pwszPath)
451+
}
419452

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

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)
474+
_ = CloseHandle(hFile)
475+
hFile = INVALID_HANDLE_VALUE
476+
477+
// The move is across volumes.
478+
guard MoveFileExW(pwszAuxiliaryPath, pwszPath, MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING) else {
479+
let dwError = GetLastError()
480+
if dwError == ERROR_ACCESS_DENIED {
481+
let dwAttributes = GetFileAttributesW(pwszPath)
482+
if dwAttributes > 0,
483+
dwAttributes & (FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM) != 0 {
484+
// The target file is read-only, hidden or a system file. Remove those attributes and try again.
485+
if SetFileAttributesW(pwszPath, dwAttributes & ~(FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM)) {
486+
if MoveFileExW(pwszAuxiliaryPath, pwszPath, MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING) {
487+
return
488+
}
489+
// Try to restore the attributes, but ignore any error
490+
_ = SetFileAttributesW(pwszPath, dwAttributes)
491+
}
492+
}
493+
}
494+
_ = DeleteFileW(pwszAuxiliaryPath)
495+
throw CocoaError.errorWithFilePath(inPath, win32: dwError, reading: false)
496+
}
497+
}
428498
}
429499
}
430500
}

Sources/FoundationEssentials/WinSDK+Extensions.swift

Lines changed: 4 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
}

0 commit comments

Comments
 (0)