Skip to content

Commit

Permalink
Move syscalls to a port of Posix enum. (#24)
Browse files Browse the repository at this point in the history
Motivation:

Right now we make a bunch of naked syscalls that don't correctly
handle reporting errno or catching EINTR. We can't use the NIO Posix
enum to fix this for us because it's internal, and because it doesn't
have the function we need. Instead, we create a temporary port of those
functions.

Ideally we'd reconcile these at some time in the future.

Modifications:

Add a port of the Posix enum and its core functions.
Improve wrapErrorIsNullReturnCall to be more useful.
Adopt the posix functions.

Result:

Better safety in the face of EINTR.
Better reporting of errnos.
  • Loading branch information
Lukasa authored Jul 12, 2018
1 parent f9a2581 commit 6617eb0
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 29 deletions.
112 changes: 112 additions & 0 deletions Sources/NIOOpenSSL/PosixPort.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

// This file contains a version of the SwiftNIO Posix enum. This is necessary
// because SwiftNIO's version is internal. Our version exists for the same reason:
// to ensure errno is captured correctly when doing syscalls, and that no ARC traffic
// can happen inbetween that *could* change the errno value before we were able to
// read it.
//
// The code is an exact port from SwiftNIO, so if that version ever becomes public we
// can lift anything missing from there and move it over without change.
import NIO

private let sysFopen: @convention(c) (UnsafePointer<CChar>?, UnsafePointer<CChar>?) -> UnsafeMutablePointer<FILE>? = fopen
private let sysMlock: @convention(c) (UnsafeRawPointer?, size_t) -> CInt = mlock
private let sysMunlock: @convention(c) (UnsafeRawPointer?, size_t) -> CInt = munlock

// Sadly, stat has different signatures with glibc and macOS libc.
#if os(Linux) || os(FreeBSD) || os(Android)
private let sysStat: @convention(c) (UnsafePointer<CChar>, UnsafeMutablePointer<stat>) -> CInt = stat(_:_:)
#elseif os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
private let sysStat: @convention(c) (UnsafePointer<CChar>?, UnsafeMutablePointer<stat>?) -> CInt = stat(_:_:)
#endif


// MARK:- Copied code from SwiftNIO
private func isBlacklistedErrno(_ code: CInt) -> Bool {
switch code {
case EFAULT, EBADF:
return true
default:
return false
}
}

/* Sorry, we really try hard to not use underscored attributes. In this case however we seem to break the inlining threshold which makes a system call take twice the time, ie. we need this exception. */
@inline(__always)
internal func wrapSyscall<T: FixedWidthInteger>(where function: StaticString = #function, _ body: () throws -> T) throws -> T {
while true {
let res = try body()
if res == -1 {
let err = errno
if err == EINTR {
continue
}
assert(!isBlacklistedErrno(err), "blacklisted errno \(err) \(strerror(err)!)")
throw IOError(errnoCode: err, function: function)
}
return res
}
}

/* Sorry, we really try hard to not use underscored attributes. In this case however we seem to break the inlining threshold which makes a system call take twice the time, ie. we need this exception. */
@inline(__always)
internal func wrapErrorIsNullReturnCall<T>(where function: StaticString = #function, _ body: () throws -> T?) throws -> T {
while true {
guard let res = try body() else {
let err = errno
if err == EINTR {
continue
}
assert(!isBlacklistedErrno(err), "blacklisted errno \(err) \(strerror(err)!)")
throw IOError(errnoCode: err, function: function)
}
return res
}
}

// MARK:- Our functions
internal enum Posix {
@inline(never)
public static func fopen(file: UnsafePointer<CChar>, mode: UnsafePointer<CChar>) throws -> UnsafeMutablePointer<FILE> {
return try wrapErrorIsNullReturnCall {
sysFopen(file, mode)
}
}

@inline(never)
@discardableResult
public static func stat(path: UnsafePointer<CChar>, buf: UnsafeMutablePointer<stat>) throws -> CInt {
return try wrapSyscall {
sysStat(path, buf)
}
}

@inline(never)
@discardableResult
public static func mlock(addr: UnsafeRawPointer, len: size_t) throws -> CInt {
return try wrapSyscall {
sysMlock(addr, len)
}
}

@inline(never)
@discardableResult
public static func munlock(addr: UnsafeRawPointer, len: size_t) throws -> CInt {
return try wrapSyscall {
sysMunlock(addr, len)
}
}
}
4 changes: 1 addition & 3 deletions Sources/NIOOpenSSL/SSLCertificate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ public class OpenSSLCertificate {
///
/// Note that this method will only ever load the first certificate from a given file.
public convenience init (file: String, format: OpenSSLSerializationFormats) throws {
guard let fileObject = fopen(file, "rb") else {
throw NIOOpenSSLError.noSuchFilesystemObject
}
let fileObject = try Posix.fopen(file: file, mode: "rb")
defer {
fclose(fileObject)
}
Expand Down
4 changes: 3 additions & 1 deletion Sources/NIOOpenSSL/SSLContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ private enum FileSystemObject {

static func pathType(path: String) -> FileSystemObject? {
var statObj = stat()
guard stat(path, &statObj) == 0 else {
do {
try Posix.stat(path: path, buf: &statObj)
} catch {
return nil
}

Expand Down
11 changes: 3 additions & 8 deletions Sources/NIOOpenSSL/SSLPKCS12Bundle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,7 @@ public struct OpenSSLPKCS12Bundle {
public init<Bytes: Collection>(file: String, passphrase: Bytes?) throws where Bytes.Element == UInt8 {
guard openSSLIsInitialized else { fatalError("Failed to initialize OpenSSL") }

guard let fileObject = fopen(file, "rb") else {
throw NIOOpenSSLError.noSuchFilesystemObject
}
let fileObject = try Posix.fopen(file: file, mode: "rb")
defer {
fclose(fileObject)
}
Expand Down Expand Up @@ -179,13 +177,10 @@ internal extension Collection where Element == UInt8 {
ptr.deallocate()
}

if mlock(bufferPtr.baseAddress!, bufferPtr.count) != 0 {
throw IOError(errnoCode: errno, function: "mlock")
}
try Posix.mlock(addr: bufferPtr.baseAddress!, len: bufferPtr.count)
defer {
// If munlock fails take out the process.
let rc = munlock(bufferPtr.baseAddress!, bufferPtr.count)
precondition(rc == 0, "munlock failed with \(rc), errno \(errno)")
try! Posix.munlock(addr: bufferPtr.baseAddress!, len: bufferPtr.count)
}

let (_, nextIndex) = bufferPtr.initialize(from: self)
Expand Down
4 changes: 1 addition & 3 deletions Sources/NIOOpenSSL/SSLPrivateKey.swift
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,7 @@ public class OpenSSLPrivateKey {

/// A delegating initializer for `init(file:format:passphraseCallback)` and `init(file:format:)`.
private convenience init(file: String, format: OpenSSLSerializationFormats, callbackManager: CallbackManagerProtocol?) throws {
guard let fileObject = fopen(file, "rb") else {
throw NIOOpenSSLError.noSuchFilesystemObject
}
let fileObject = try Posix.fopen(file: file, mode: "rb")
defer {
fclose(fileObject)
}
Expand Down
8 changes: 4 additions & 4 deletions Tests/NIOOpenSSLTests/SSLCertificateTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,8 @@ class SSLCertificateTest: XCTestCase {
do {
_ = try OpenSSLCertificate(file: "/nonexistent/path", format: .pem)
XCTFail("Did not throw")
} catch NIOOpenSSLError.noSuchFilesystemObject {
// ok
} catch let error as IOError {
XCTAssertEqual(error.errnoCode, ENOENT)
} catch {
XCTFail("Unexpected error: \(error)")
}
Expand All @@ -262,8 +262,8 @@ class SSLCertificateTest: XCTestCase {
do {
_ = try OpenSSLCertificate(file: "/nonexistent/path", format: .der)
XCTFail("Did not throw")
} catch NIOOpenSSLError.noSuchFilesystemObject {
// ok
} catch let error as IOError {
XCTAssertEqual(error.errnoCode, ENOENT)
} catch {
XCTFail("Unexpected error: \(error)")
}
Expand Down
4 changes: 2 additions & 2 deletions Tests/NIOOpenSSLTests/SSLPKCS12BundleTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,8 @@ class SSLPKCS12BundleTest: XCTestCase {
do {
_ = try OpenSSLPKCS12Bundle(file: "/nonexistent/path")
XCTFail("Did not throw")
} catch NIOOpenSSLError.noSuchFilesystemObject {
// Ok
} catch let error as IOError {
XCTAssertEqual(error.errnoCode, ENOENT)
} catch {
XCTFail("Unexpected error: \(error)")
}
Expand Down
16 changes: 8 additions & 8 deletions Tests/NIOOpenSSLTests/SSLPrivateKeyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ class SSLPrivateKeyTest: XCTestCase {
do {
_ = try OpenSSLPrivateKey(file: "/nonexistent/path", format: .pem)
XCTFail("Did not throw")
} catch NIOOpenSSLError.noSuchFilesystemObject {
// ok
} catch let error as IOError {
XCTAssertEqual(error.errnoCode, ENOENT)
} catch {
XCTFail("Unexpected error: \(error)")
}
Expand All @@ -152,8 +152,8 @@ class SSLPrivateKeyTest: XCTestCase {
do {
_ = try OpenSSLPrivateKey(file: "/nonexistent/path", format: .der)
XCTFail("Did not throw")
} catch NIOOpenSSLError.noSuchFilesystemObject {
// ok
} catch let error as IOError {
XCTAssertEqual(error.errnoCode, ENOENT)
} catch {
XCTFail("Unexpected error: \(error)")
}
Expand All @@ -165,8 +165,8 @@ class SSLPrivateKeyTest: XCTestCase {
XCTFail("Should not be called")
}
XCTFail("Did not throw")
} catch NIOOpenSSLError.noSuchFilesystemObject {
// ok
} catch let error as IOError {
XCTAssertEqual(error.errnoCode, ENOENT)
} catch {
XCTFail("Unexpected error: \(error)")
}
Expand All @@ -178,8 +178,8 @@ class SSLPrivateKeyTest: XCTestCase {
XCTFail("Should not be called")
}
XCTFail("Did not throw")
} catch NIOOpenSSLError.noSuchFilesystemObject {
// ok
} catch let error as IOError {
XCTAssertEqual(error.errnoCode, ENOENT)
} catch {
XCTFail("Unexpected error: \(error)")
}
Expand Down

0 comments on commit 6617eb0

Please sign in to comment.