Skip to content

Commit

Permalink
Not retry calling close(...) when EINTR is reported. (#217)
Browse files Browse the repository at this point in the history
Motivation:

Usually we retry the syscall when EINTR is reported. This is not safe in the case of close(...) as the file descriptor may already be closed. The problem arise if this was the case and we retry it and the file descriptor is already reused again. In this case we would close a wrong file descriptor.

See:
  - https://bugs.chromium.org/p/chromium/issues/detail?id=269623
  - https://lwn.net/Articles/576478/

Modifications:

Ignore EINTR on close(...) and just assume the FD was closed.

Result:

Not possible to close the wrong file descriptor when close(...) reports EINTR.
  • Loading branch information
normanmaurer authored Mar 22, 2018
1 parent 1f83a1d commit 91d7058
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions Sources/NIO/System.swift
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,20 @@ internal enum Posix {

@inline(never)
public static func close(descriptor: CInt) throws {
_ = try wrapSyscall {
sysClose(descriptor)
let res = sysClose(descriptor)
if res == -1 {
let err = errno

// There is really nothing "sane" we can do when EINTR was reported on close.
// So just ignore it and "assume" everything is fine == we closed the file descriptor.
//
// For more details see:
// - https://bugs.chromium.org/p/chromium/issues/detail?id=269623
// - https://lwn.net/Articles/576478/
if err != EINTR {
assert(!isBlacklistedErrno(err), "blacklisted errno \(err) \(strerror(err)!)")
throw IOError(errnoCode: err, function: "close")
}
}
}

Expand Down

0 comments on commit 91d7058

Please # to comment.