Skip to content

Commit

Permalink
Avoid double-closing on fcntl failures (#2409)
Browse files Browse the repository at this point in the history
Motivation:

The fix provided in #2407 was subtly wrong. ignoreSIGPIPE, which throws
the error in question, closes the FD on error _except_ on EINVAL from
fcntl, where it instead does not. This inconsistent behaviour is the
source of the bug. Because this behaviour is inconsistent, the fix from
PR #2407 is also inconsistent and can in some cases double-close the
socket.

The actual issue is not as old as I expected: the code can be observed
by reviewing the change in #1598, which incorrectly inserted the error
transformation before the call to close.

Modifications:

- Revert the change from #2407.
- Move the close in ignoreSIGPIPE to before the error check, rather than
  after, so we unconditionally execute it.

Result:

More resilient fix.
  • Loading branch information
Lukasa authored Apr 20, 2023
1 parent 003fbad commit f7c4655
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 9 deletions.
9 changes: 1 addition & 8 deletions Sources/NIOPosix/ServerSocket.swift
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,7 @@ import NIOCore
return nil
}

let sock: Socket
do {
sock = try Socket(socket: fd)
} catch {
// best effort
try? Posix.close(descriptor: fd)
throw error
}
let sock = try Socket(socket: fd)

#if !os(Linux)
if setNonBlocking {
Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOPosix/SocketProtocols.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ extension BaseSocketProtocol {
do {
try Posix.fcntl(descriptor: fd, command: F_SETNOSIGPIPE, value: 1)
} 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
Expand Down

0 comments on commit f7c4655

Please sign in to comment.