mirror of
https://github.com/apple/container.git
synced 2026-09-10 09:45:39 +00:00
Eliminate race windows in the Socket class (#245)
While individual accesses were properly mutex-protected, the logical operations were not atomic, leading to a potential inconsistency in a multi-threaded environment. Multiple separate `state.withLock` calls created race windows. With the proposed changes, single atomic operations check and update the state together.
This commit is contained in:
Dmitry Kovba
@@ -115,95 +115,100 @@ extension Socket {
|
||||
}
|
||||
|
||||
public func connect() throws {
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
try state.withLock { currentState in
|
||||
guard currentState.socketState == .created else {
|
||||
throw SocketError.invalidOperationOnSocket("connect")
|
||||
}
|
||||
guard let handle = currentState.handle else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
|
||||
guard state.withLock({ $0.socketState }) == .created else {
|
||||
throw SocketError.invalidOperationOnSocket("connect")
|
||||
}
|
||||
|
||||
var res: Int32 = 0
|
||||
try state.withLock {
|
||||
try $0.type.withSockAddr { (ptr, length) in
|
||||
var res: Int32 = 0
|
||||
try currentState.type.withSockAddr { (ptr, length) in
|
||||
res = Syscall.retrying {
|
||||
sysConnect(handle.fileDescriptor, ptr, length)
|
||||
}
|
||||
}
|
||||
}
|
||||
if res == -1 {
|
||||
throw Socket.errnoToError(msg: "could not connect to socket \(state.withLock { $0.type })")
|
||||
}
|
||||
state.withLock {
|
||||
$0 = State(
|
||||
|
||||
if res == -1 {
|
||||
throw Socket.errnoToError(msg: "could not connect to socket \(currentState.type)")
|
||||
}
|
||||
|
||||
currentState = State(
|
||||
socketState: .connected,
|
||||
handle: handle,
|
||||
type: $0.type,
|
||||
acceptSource: $0.acceptSource
|
||||
type: currentState.type,
|
||||
acceptSource: currentState.acceptSource
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
public func listen() throws {
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
guard state.withLock({ $0.socketState }) == .created else {
|
||||
throw SocketError.invalidOperationOnSocket("listen")
|
||||
}
|
||||
try state.withLock { currentState in
|
||||
guard currentState.socketState == .created else {
|
||||
throw SocketError.invalidOperationOnSocket("listen")
|
||||
}
|
||||
guard let handle = currentState.handle else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
|
||||
try state.withLock { try $0.type.beforeBind(fd: handle.fileDescriptor) }
|
||||
try currentState.type.beforeBind(fd: handle.fileDescriptor)
|
||||
|
||||
var rc: Int32 = 0
|
||||
try state.withLock {
|
||||
try $0.type.withSockAddr { (ptr, length) in
|
||||
var rc: Int32 = 0
|
||||
try currentState.type.withSockAddr { (ptr, length) in
|
||||
rc = sysBind(handle.fileDescriptor, ptr, length)
|
||||
}
|
||||
}
|
||||
if rc < 0 {
|
||||
throw Socket.errnoToError(msg: "could not bind to \(state.withLock { $0.type })")
|
||||
}
|
||||
|
||||
try state.withLock { try $0.type.beforeListen(fd: handle.fileDescriptor) }
|
||||
if sysListen(handle.fileDescriptor, SOMAXCONN) < 0 {
|
||||
throw Socket.errnoToError(msg: "listen failed on \(state.withLock { $0.type })")
|
||||
}
|
||||
state.withLock {
|
||||
$0 = State(
|
||||
if rc < 0 {
|
||||
throw Socket.errnoToError(msg: "could not bind to \(currentState.type)")
|
||||
}
|
||||
|
||||
try currentState.type.beforeListen(fd: handle.fileDescriptor)
|
||||
|
||||
if sysListen(handle.fileDescriptor, SOMAXCONN) < 0 {
|
||||
throw Socket.errnoToError(msg: "listen failed on \(currentState.type)")
|
||||
}
|
||||
|
||||
currentState = State(
|
||||
socketState: .listening,
|
||||
handle: handle,
|
||||
type: $0.type,
|
||||
acceptSource: $0.acceptSource
|
||||
type: currentState.type,
|
||||
acceptSource: currentState.acceptSource
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
public func close() throws {
|
||||
// Already closed.
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
return
|
||||
}
|
||||
if let acceptSource = state.withLock({ $0.acceptSource }) {
|
||||
acceptSource.cancel()
|
||||
}
|
||||
try handle.close()
|
||||
state.withLock {
|
||||
$0 = State(
|
||||
socketState: $0.socketState,
|
||||
let (handleToClose, sourceToCancel) = state.withLock { currentState -> (FileHandle?, DispatchSourceRead?) in
|
||||
guard let handle = currentState.handle else {
|
||||
// Already closed.
|
||||
return (nil, nil)
|
||||
}
|
||||
|
||||
currentState = State(
|
||||
socketState: currentState.socketState,
|
||||
handle: nil,
|
||||
type: $0.type,
|
||||
type: currentState.type,
|
||||
acceptSource: nil
|
||||
)
|
||||
|
||||
return (handle, currentState.acceptSource)
|
||||
}
|
||||
|
||||
// Close outside the lock to avoid a deadlock.
|
||||
sourceToCancel?.cancel()
|
||||
try handleToClose?.close()
|
||||
}
|
||||
|
||||
public func write(data: any DataProtocol) throws -> Int {
|
||||
guard state.withLock({ $0.socketState }) == .connected else {
|
||||
throw SocketError.invalidOperationOnSocket("write")
|
||||
}
|
||||
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
throw SocketError.closed
|
||||
let handle = try state.withLock { currentState in
|
||||
guard currentState.socketState == .connected else {
|
||||
throw SocketError.invalidOperationOnSocket("write")
|
||||
}
|
||||
guard let handle = currentState.handle else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
return handle
|
||||
}
|
||||
|
||||
if data.isEmpty {
|
||||
@@ -215,29 +220,29 @@ extension Socket {
|
||||
}
|
||||
|
||||
public func acceptStream(closeOnDeinit: Bool = true) throws -> AsyncThrowingStream<Socket, Swift.Error> {
|
||||
guard state.withLock({ $0.socketState }) == .listening else {
|
||||
throw SocketError.invalidOperationOnSocket("accept")
|
||||
}
|
||||
let source = try state.withLock { currentState -> DispatchSourceRead in
|
||||
guard currentState.socketState == .listening else {
|
||||
throw SocketError.invalidOperationOnSocket("accept")
|
||||
}
|
||||
guard let handle = currentState.handle else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
guard currentState.acceptSource == nil else {
|
||||
throw SocketError.acceptStreamExists
|
||||
}
|
||||
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
|
||||
guard state.withLock({ $0.acceptSource }) == nil else {
|
||||
throw SocketError.acceptStreamExists
|
||||
}
|
||||
|
||||
let source = state.withLock {
|
||||
let source = DispatchSource.makeReadSource(
|
||||
fileDescriptor: handle.fileDescriptor,
|
||||
queue: _queue
|
||||
)
|
||||
$0 = State(
|
||||
socketState: $0.socketState,
|
||||
|
||||
currentState = State(
|
||||
socketState: currentState.socketState,
|
||||
handle: handle,
|
||||
type: $0.type,
|
||||
type: currentState.type,
|
||||
acceptSource: source
|
||||
)
|
||||
|
||||
return source
|
||||
}
|
||||
|
||||
@@ -266,29 +271,33 @@ extension Socket {
|
||||
}
|
||||
|
||||
public func accept(closeOnDeinit: Bool = true) throws -> Socket {
|
||||
guard state.withLock({ $0.socketState }) == .listening else {
|
||||
throw SocketError.invalidOperationOnSocket("accept")
|
||||
let (handle, socketType) = try state.withLock { currentState in
|
||||
guard currentState.socketState == .listening else {
|
||||
throw SocketError.invalidOperationOnSocket("accept")
|
||||
}
|
||||
guard let handle = currentState.handle else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
return (handle, currentState.type)
|
||||
}
|
||||
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
|
||||
let (clientFD, socketType) = try state.withLock { try $0.type.accept(fd: handle.fileDescriptor) }
|
||||
let (clientFD, newSocketType) = try socketType.accept(fd: handle.fileDescriptor)
|
||||
return Socket(
|
||||
fd: clientFD,
|
||||
type: socketType,
|
||||
type: newSocketType,
|
||||
closeOnDeinit: closeOnDeinit
|
||||
)
|
||||
}
|
||||
|
||||
public func read(buffer: inout Data) throws -> Int {
|
||||
guard state.withLock({ $0.socketState }) == .connected else {
|
||||
throw SocketError.invalidOperationOnSocket("read")
|
||||
}
|
||||
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
throw SocketError.closed
|
||||
let handle = try state.withLock { currentState in
|
||||
guard currentState.socketState == .connected else {
|
||||
throw SocketError.invalidOperationOnSocket("read")
|
||||
}
|
||||
guard let handle = currentState.handle else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
return handle
|
||||
}
|
||||
|
||||
var bytesRead = 0
|
||||
@@ -311,8 +320,11 @@ extension Socket {
|
||||
}
|
||||
|
||||
public func shutdown(how: ShutdownOption) throws {
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
throw SocketError.closed
|
||||
let handle = try state.withLock { currentState in
|
||||
guard let handle = currentState.handle else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
return handle
|
||||
}
|
||||
|
||||
var howOpt: Int32 = 0
|
||||
@@ -331,17 +343,24 @@ extension Socket {
|
||||
}
|
||||
|
||||
public func setSockOpt(sockOpt: Int32 = 0, ptr: UnsafeRawPointer, stride: UInt32) throws {
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
throw SocketError.closed
|
||||
let handle = try state.withLock { currentState in
|
||||
guard let handle = currentState.handle else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
return handle
|
||||
}
|
||||
|
||||
if setsockopt(handle.fileDescriptor, SOL_SOCKET, sockOpt, ptr, stride) < 0 {
|
||||
throw Socket.errnoToError(msg: "failed to set sockopt")
|
||||
}
|
||||
}
|
||||
|
||||
public func setTimeout(option: TimeoutOption, seconds: Int) throws {
|
||||
guard let handle = state.withLock({ $0.handle }) else {
|
||||
throw SocketError.closed
|
||||
let handle = try state.withLock { currentState in
|
||||
guard let handle = currentState.handle else {
|
||||
throw SocketError.closed
|
||||
}
|
||||
return handle
|
||||
}
|
||||
|
||||
var sockOpt: Int32 = 0
|
||||
|
||||
Reference in New Issue
Block a user