Add a fallback path if renameat2 fails (#2733)

Motivation:

'renameat2' can fail with EINVAL if the RENAME_NOREPLACE flag isn't
supported. However, not all kernel versions support this flag which can
result in an error when creating a file 'transactionally'.

Using 'renameat2' only happens on Linux as a fallback when O_TMPFILE
isn't available.

Modifications:

- On Linux, fallback to a combination of 'stat' and 'rename' if 'renameat2' fails with EINVAL.
- Add a couple of tests

Result:

Files can still be created exclusively

Author: glbrntt

Sources/NIOFileSystem/Internal/SystemFileHandle.swift

@@ -650,7 +650,7 @@ extension SystemFileHandle.SendableView {
     }
 
     @_spi(Testing)
-    public func _close(materialize: Bool) -> Result<Void, FileSystemError> {
+    public func _close(materialize: Bool, failRenameat2WithEINVAL: Bool = false) -> Result<Void, FileSystemError> {
         let descriptor: FileDescriptor? = self.lifecycle.withLockedValue { lifecycle in
             switch lifecycle {
             case let .open(descriptor):
@@ -666,7 +666,7 @@ extension SystemFileHandle.SendableView {
         }
 
         // Materialize then close.
-        let materializeResult = self._materialize(materialize, descriptor: descriptor)
+        let materializeResult = self._materialize(materialize, descriptor: descriptor, failRenameat2WithEINVAL: failRenameat2WithEINVAL)
 
         return Result {
             try descriptor.close()
@@ -778,7 +778,8 @@ extension SystemFileHandle.SendableView {
 
     func _materialize(
         _ materialize: Bool,
-        descriptor: FileDescriptor
+        descriptor: FileDescriptor,
+        failRenameat2WithEINVAL: Bool
     ) -> Result<Void, FileSystemError> {
         guard let materialization = self.materialization else { return .success(()) }
 
@@ -803,7 +804,7 @@ extension SystemFileHandle.SendableView {
 
         case .rename:
             if materialize {
-                let renameResult: Result<Void, Errno>
+                var renameResult: Result<Void, Errno>
                 let renameFunction: String
                 #if canImport(Darwin)
                 renameFunction = "renamex_np"
@@ -817,19 +818,76 @@ extension SystemFileHandle.SendableView {
                 // ignored. However, they must still be provided to 'rename' in order to pass
                 // flags.
                 renameFunction = "renameat2"
-                renameResult = Syscall.rename(
-                    from: createdPath,
-                    relativeTo: .currentWorkingDirectory,
-                    to: desiredPath,
-                    relativeTo: .currentWorkingDirectory,
-                    flags: materialization.exclusive ? [.exclusive] : []
-                )
+                if materialization.exclusive, failRenameat2WithEINVAL {
+                    renameResult = .failure(.invalidArgument)
+                } else {
+                    renameResult = Syscall.rename(
+                        from: createdPath,
+                        relativeTo: .currentWorkingDirectory,
+                        to: desiredPath,
+                        relativeTo: .currentWorkingDirectory,
+                        flags: materialization.exclusive ? [.exclusive] : []
+                    )
+                }
                 #endif
 
-                // A file exists at the desired path and the user specified exclusive creation,
-                // clear up by removing the file we did create.
-                if materialization.exclusive, case .failure(.fileExists) = renameResult {
-                    _ = Libc.remove(createdPath)
+                if materialization.exclusive {
+                    switch renameResult {
+                    case .failure(.fileExists):
+                        // A file exists at the desired path and the user specified exclusive
+                        // creation, clear up by removing the file that we did create.
+                        _ = Libc.remove(createdPath)
+
+                    case .failure(.invalidArgument):
+                        // If 'renameat2' failed on Linux with EINVAL then in all likelihood the
+                        // 'RENAME_NOREPLACE' option isn't supported. As we're doing an exclusive
+                        // create, check the desired path doesn't exist then do a regular rename.
+                        #if canImport(Glibc) || canImport(Musl)
+                        switch Syscall.stat(path: desiredPath) {
+                        case .failure(.noSuchFileOrDirectory):
+                            // File doesn't exist, do a 'regular' rename.
+                            renameResult = Syscall.rename(from: createdPath, to: desiredPath)
+
+                        case .success:
+                            // File exists so exclusive create isn't possible. Remove the file
+                            // we did create then throw.
+                            _ = Libc.remove(createdPath)
+                            let error = FileSystemError(
+                                code: .fileAlreadyExists,
+                                message: """
+                                    Couldn't open '\(desiredPath)', it already exists and the \
+                                    file was opened with the 'existingFile' option set to 'none'.
+                                    """,
+                                cause: nil,
+                                location: .here()
+                            )
+                            return .failure(error)
+
+                        case .failure:
+                            // Failed to stat the desired file for reasons unknown. Remove the file
+                            // we did create then throw.
+                            _ = Libc.remove(createdPath)
+                            let error = FileSystemError(
+                                code: .unknown,
+                                message: "Couldn't open '\(desiredPath)'.",
+                                cause: FileSystemError.rename(
+                                    "renameat2",
+                                    errno: .invalidArgument,
+                                    oldName: createdPath,
+                                    newName: desiredPath,
+                                    location: .here()
+                                ),
+                                location: .here()
+                            )
+                            return .failure(error)
+                        }
+                        #else
+                        ()  // Not Linux, use the normal error flow.
+                        #endif
+
+                    case .success, .failure:
+                        ()
+                    }
                 }
 
                 result = renameResult.mapError { errno in
@@ -1238,7 +1296,8 @@ extension SystemFileHandle {
         }
     }
 
-    static func syncOpenWithMaterialization(
+    @_spi(Testing)
+    public static func syncOpenWithMaterialization(
         atPath path: FilePath,
         mode: FileDescriptor.AccessMode,
         options originalOptions: FileDescriptor.OpenOptions,

Tests/NIOFileSystemIntegrationTests/FileHandleTests.swift

@@ -995,6 +995,55 @@ final class FileHandleTests: XCTestCase {
         }
     }
 
+    func testOpenExclusiveCreateForFileWhichExistsWithoutOTMPFILE() async throws {
+        // Takes the path where 'O_TMPFILE' doesn't exist, so materializing the file is done via
+        // creating a temporary file and then renaming it using 'renamex_np'/'renameat2' (Darwin/Linux).
+        let temporaryDirectory = try await FileSystem.shared.temporaryDirectory
+        let path = temporaryDirectory.appending(Self.temporaryFileName().components)
+        let handle = try SystemFileHandle.syncOpenWithMaterialization(
+            atPath: path,
+            mode: .writeOnly,
+            options: [.exclusiveCreate, .create],
+            permissions: .ownerReadWrite,
+            threadPool: .singleton,
+            useTemporaryFileIfPossible: false
+        ).get()
+
+        // Closing shouldn't throw and the file should now be visible.
+        try await handle.close()
+        let info = try await FileSystem.shared.info(forFileAt: path)
+        XCTAssertNotNil(info)
+    }
+
+    func testOpenExclusiveCreateForFileWhichExistsWithoutOTMPFILEOrRenameat2() async throws {
+        // Takes the path where 'O_TMPFILE' doesn't exist, so materializing the file is done via
+        // creating a temporary file and then renaming it using 'renameat2' and then takes a further
+        // fallback path where 'renameat2' returns EINVAL so the 'rename' is used in combination
+        // with 'stat'. This path is only reachable on Linux.
+        #if canImport(Glibc) || canImport(Musl)
+        let temporaryDirectory = try await FileSystem.shared.temporaryDirectory
+        let path = temporaryDirectory.appending(Self.temporaryFileName().components)
+        let handle = try SystemFileHandle.syncOpenWithMaterialization(
+            atPath: path,
+            mode: .writeOnly,
+            options: [.exclusiveCreate, .create],
+            permissions: .ownerReadWrite,
+            threadPool: .singleton,
+            useTemporaryFileIfPossible: false
+        ).get()
+
+        // Close, but take the path where 'renameat2' fails with EINVAL. This shouldn't throw and
+        // the file should be available.
+        let result = handle.sendableView._close(materialize: true, failRenameat2WithEINVAL: true)
+        try result.get()
+
+        let info = try await FileSystem.shared.info(forFileAt: path)
+        XCTAssertNotNil(info)
+        #else
+        throw XCTSkip("This test requires 'renameat2' which isn't supported on this platform")
+        #endif
+    }
+
     func testOpenExclusiveCreateForFileWhichDoesNotExist() async throws {
         try await self.withTestDataDirectory { dir in
             let path = Self.temporaryFileName()