Avoid double-closing on fcntl failures (#2409)

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.
This commit is contained in:
Cory Benfield 2023-04-20 12:40:39 +01:00 committed by GitHub
parent 003fbadf51
commit f7c4655298
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 2 additions and 9 deletions

View File

@ -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 {

View File

@ -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