From 74bbadbc7abf220f5c583750d92190202abb20eb Mon Sep 17 00:00:00 2001 From: Jim Dovey Date: Thu, 22 Feb 2018 12:42:08 -0800 Subject: [PATCH 1/3] Fixed a bit-addressing error in fd_set implementation. - Now uses an unsigned type to perform the bit-shift operation and then casts to signed type using `init(bitPattern:)`. - Added a unit test case which ensures fd values from zero to (arbitrarily) 128 can all be inserted and removed from the fd_set. --- Sources/Socket/SocketUtils.swift | 2 +- Tests/SocketTests/SocketTests.swift | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Sources/Socket/SocketUtils.swift b/Sources/Socket/SocketUtils.swift index 384b9ab..2990d7a 100644 --- a/Sources/Socket/SocketUtils.swift +++ b/Sources/Socket/SocketUtils.swift @@ -154,7 +154,7 @@ public extension fd_set { private static func address(for fd: Int32) -> (Int, Int32) { let intOffset = Int(fd) / __fd_set_count let bitOffset = Int(fd) % __fd_set_count - let mask = Int32(1 << bitOffset) + let mask = Int32(bitPattern: UInt32(1 << bitOffset)) return (intOffset, mask) } diff --git a/Tests/SocketTests/SocketTests.swift b/Tests/SocketTests/SocketTests.swift index 308d8e4..ac4a510 100644 --- a/Tests/SocketTests/SocketTests.swift +++ b/Tests/SocketTests/SocketTests.swift @@ -1125,6 +1125,18 @@ class SocketTests: XCTestCase { } } + func testFDSetBitFields() { + var fdSet = fd_set() + fdSet.zero() + + for i: Int32 in 0...128 { + fdSet.set(i) + XCTAssertTrue(fdSet.isSet(i)) + fdSet.clear(i) + XCTAssertFalse(fdSet.isSet(i)) + } + } + func testReadWrite() { let hostname = "127.0.0.1" From c8ee463a06c32718f2ba8f59c259ec81e28ccb7d Mon Sep 17 00:00:00 2001 From: Jim Dovey Date: Thu, 22 Feb 2018 13:08:31 -0800 Subject: [PATCH 2/3] Makes sure that the temporary socket opened in `Socket.connect(to:port:timeout:)` is closed in the event of an error being thrown. I've added two pieces here: first is a `defer` block right after the creation of the `socketDescriptor` variable. If it's non-nil and not `Socket.SOCKET_INVALID_DESCRIPTOR` then the socket is closed pronto. Secondly, I'm explicitly setting this variable to `nil` when it's copied into `self.socketfd`. From that point on, any failure or error thrown will not result in the socket being orphaned, since it's referenced by the `Socket` class instance, and will be cleaned up in its `deinit` implementation. --- Sources/Socket/Socket.swift | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Sources/Socket/Socket.swift b/Sources/Socket/Socket.swift index b5f47f0..ba288b1 100644 --- a/Sources/Socket/Socket.swift +++ b/Sources/Socket/Socket.swift @@ -1683,6 +1683,17 @@ public class Socket: SocketReader, SocketWriter { } var socketDescriptor: Int32? + defer { + // if we throw an error, be sure we clean up any dangling socket properly. + // note that we set this variable to nil when we assign the socket to `self.socketfd`. + if let sock = socketDescriptor, sock != Socket.SOCKET_INVALID_DESCRIPTOR { + #if os(Linux) + _ = Glibc.close(socketDescriptor!) + #else + _ = Darwin.close(socketDescriptor!) + #endif + } + } var info = targetInfo while info != nil { @@ -1817,6 +1828,8 @@ public class Socket: SocketReader, SocketWriter { } self.socketfd = socketDescriptor! + socketDescriptor = nil // clear out the temporary value -- our defer() can check for that alone + self.isConnected = true var address: Address if info!.pointee.ai_family == Int32(AF_INET6) { @@ -1851,7 +1864,7 @@ public class Socket: SocketReader, SocketWriter { // Socket supposed to be blocking but we've changed it to non-blocking because // a timeout was requested... Got to change it back before proceeding... - let flags = fcntl(socketDescriptor!, F_GETFL) + let flags = fcntl(self.socketfd, F_GETFL) if flags < 0 { throw Error(code: Socket.SOCKET_ERR_GET_FCNTL_FAILED, reason: self.lastError()) From ec3b6f5d5599ea3b1ca3d6d817a864a9fc83c19a Mon Sep 17 00:00:00 2001 From: Jim Dovey Date: Thu, 22 Feb 2018 14:33:24 -0800 Subject: [PATCH 3/3] Adds the new unit test to the `allTests` array. --- Tests/SocketTests/SocketTests.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/SocketTests/SocketTests.swift b/Tests/SocketTests/SocketTests.swift index ac4a510..544741c 100644 --- a/Tests/SocketTests/SocketTests.swift +++ b/Tests/SocketTests/SocketTests.swift @@ -1519,6 +1519,7 @@ class SocketTests: XCTestCase { ("testSetWriteTimeout", testSetWriteTimeout), ("testIsReadableWritableFail", testIsReadableWritableFail), ("testIsReadableWritable", testIsReadableWritable), + ("testFDSetBitFields", testFDSetBitFields), ("testReadWrite", testReadWrite), ("testTruncateTCP", testTruncateTCP), ("testReadWriteUDP", testReadWriteUDP),