Recover from EINVAL even if the error is untyped (#1598)

* Recover from EINVAL even if the error is untyped

* Handle fcntl EINVAL error

* Fix EINVAL test

* Rename and deprecate errors

Co-authored-by: Cory Benfield <lukasa@apple.com>

Sources/NIO/FileDescriptor.swift

@@ -41,7 +41,7 @@ extension FileDescriptor {
         } catch let error as IOError {
             if error.errnoCode == EINVAL {
                 // Darwin seems to sometimes do this despite the docs claiming it can't happen
-                throw NIOFailedToSetSocketNonBlockingError()
+                throw NIOFcntlFailedError()
             }
             throw error
         }

Sources/NIO/SocketChannel.swift

@@ -249,9 +249,12 @@ final class ServerSocketChannel: BaseSocketChannel<ServerSocket> {
     }
 
     override func shouldCloseOnReadError(_ err: Error) -> Bool {
-        if err is NIOFailedToSetSocketNonBlockingError {
+        if err is NIOFcntlFailedError {
-            // see https://github.com/apple/swift-nio/issues/1030
+            // See:
-            // on Darwin, fcntl(fd, F_SETFL, O_NONBLOCK) sometimes returns EINVAL...
+            // - https://github.com/apple/swift-nio/issues/1030
+            // - https://github.com/apple/swift-nio/issues/1598
+            // on Darwin, fcntl(fd, F_SETFL, O_NONBLOCK) or fcntl(fd, F_SETNOSIGPIPE)
+            // sometimes returns EINVAL...
             return false
         }
         guard let err = err as? IOError else { return true }

Sources/NIO/SocketProtocols.swift

@@ -89,8 +89,12 @@ extension BaseSocketProtocol {
         assert(fd >= 0, "illegal file descriptor \(fd)")
         do {
             try Posix.fcntl(descriptor: fd, command: F_SETNOSIGPIPE, value: 1)
-        } catch {
+        } catch let error as IOError {
-            _ = try? Posix.close(descriptor: fd) // don't care about failure here
+            if error.errnoCode == EINVAL {
+                // Darwin seems to sometimes do this despite the docs claiming it can't happen
+                throw NIOFcntlFailedError()
+            }
+            try? Posix.close(descriptor: fd) // don't care about failure here
             throw error
         }
         #endif

Sources/NIO/System.swift

@@ -300,19 +300,7 @@ internal enum Posix {
                               addr: UnsafeMutablePointer<sockaddr>?,
                               len: UnsafeMutablePointer<socklen_t>?) throws -> CInt? {
         let result: IOResult<CInt> = try syscall(blocking: true) {
-            let fd = sysAccept(descriptor, addr, len)
+            return sysAccept(descriptor, addr, len)
-
-            #if !os(Linux)
-                if fd != -1 {
-                    do {
-                        try Posix.fcntl(descriptor: fd, command: F_SETNOSIGPIPE, value: 1)
-                    } catch {
-                        _ = sysClose(fd) // don't care about failure here
-                        throw error
-                    }
-                }
-            #endif
-            return fd
         }
 
         if case .processed(let fd) = result {
@@ -574,11 +562,18 @@ internal enum Posix {
     }
 }
 
+/// `NIOFcntlFailedError` indicates that NIO was unable to perform an
+/// operation on a socket.
+///
+/// This error should never happen, unfortunately, we have seen this happen on Darwin.
+public struct NIOFcntlFailedError: Error {}
+
 /// `NIOFailedToSetSocketNonBlockingError` indicates that NIO was unable to set a socket to non-blocking mode, either
 /// when connecting a socket as a client or when accepting a socket as a server.
 ///
 /// This error should never happen because a socket should always be able to be set to non-blocking mode. Unfortunately,
 /// we have seen this happen on Darwin.
+@available(*, deprecated, renamed: "NIOFcntlFailedError")
 public struct NIOFailedToSetSocketNonBlockingError: Error {}
 
 internal extension Posix {
@@ -590,7 +585,7 @@ internal extension Posix {
         } catch let error as IOError {
             if error.errnoCode == EINVAL {
                 // Darwin seems to sometimes do this despite the docs claiming it can't happen
-                throw NIOFailedToSetSocketNonBlockingError()
+                throw NIOFcntlFailedError()
             }
             throw error
         }

Tests/NIOTests/SocketChannelTest.swift

@@ -594,9 +594,9 @@ public final class SocketChannelTest : XCTestCase {
             serverChannel.read()
         
             // Wait for the server to have something
-            let result = try assertNoThrowWithValue(serverPromise.futureResult.wait())
+            XCTAssertThrowsError(try serverPromise.futureResult.wait()) { error in
-        
+                XCTAssert(error is NIOFcntlFailedError)
-            XCTAssertEqual(result.errnoCode, EINVAL)
+            }
         #endif
     }
 
@@ -665,13 +665,15 @@ public final class SocketChannelTest : XCTestCase {
     }
 
     func testServerChannelDoesNotBreakIfAcceptingFailsWithEINVAL() throws {
-        // regression test for https://github.com/apple/swift-nio/issues/1030
+        // regression test for:
+        // - https://github.com/apple/swift-nio/issues/1030
+        // - https://github.com/apple/swift-nio/issues/1598
         class HandsOutMoodySocketsServerSocket: ServerSocket {
             let shouldAcceptsFail: NIOAtomic<Bool> = .makeAtomic(value: true)
             override func accept(setNonBlocking: Bool = false) throws -> Socket? {
                 XCTAssertTrue(setNonBlocking)
                 if self.shouldAcceptsFail.load() {
-                    throw NIOFailedToSetSocketNonBlockingError()
+                    throw NIOFcntlFailedError()
                 } else {
                     return try Socket(protocolFamily: .inet,
                                       type: .stream,
@@ -688,7 +690,7 @@ public final class SocketChannelTest : XCTestCase {
             }
 
             func errorCaught(context: ChannelHandlerContext, error: Error) {
-                XCTAssert(error is NIOFailedToSetSocketNonBlockingError, "unexpected error: \(error)")
+                XCTAssert(error is NIOFcntlFailedError, "unexpected error: \(error)")
             }
         }