diff --git a/.swift-format b/.swift-format index e458d31e83..7fa06fb305 100644 --- a/.swift-format +++ b/.swift-format @@ -49,7 +49,7 @@ "OrderedImports" : true, "ReplaceForEachWithForLoop" : true, "ReturnVoidInsteadOfEmptyTuple" : true, - "UseEarlyExits" : true, + "UseEarlyExits" : false, "UseExplicitNilCheckInConditions" : false, "UseLetInEveryBoundCaseVariable" : false, "UseShorthandTypeNames" : true, diff --git a/IntegrationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift b/IntegrationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift index b75ce037eb..ebcad8f0b5 100644 --- a/IntegrationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift +++ b/IntegrationTests/tests_04_performance/test_01_resources/test_ping_pong_1000_reqs_1_conn.swift @@ -27,11 +27,12 @@ private final class PongDecoder: ByteToMessageDecoder { typealias InboundOut = UInt8 public func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) -> DecodingState { - guard let ping = buffer.readInteger(as: UInt8.self) else { + if let ping = buffer.readInteger(as: UInt8.self) { + context.fireChannelRead(Self.wrapInboundOut(ping)) + return .continue + } else { return .needMoreData } - context.fireChannelRead(Self.wrapInboundOut(ping)) - return .continue } public func decodeLast( diff --git a/Sources/NIOConcurrencyHelpers/atomics.swift b/Sources/NIOConcurrencyHelpers/atomics.swift index 43e456584d..1eb276c8c9 100644 --- a/Sources/NIOConcurrencyHelpers/atomics.swift +++ b/Sources/NIOConcurrencyHelpers/atomics.swift @@ -501,11 +501,12 @@ public final class AtomicBox { return true } else { let currentPtrBits = self.storage.load() - guard currentPtrBits == 0 || currentPtrBits == expectedPtrBits else { + if currentPtrBits == 0 || currentPtrBits == expectedPtrBits { + sys_sched_yield() + continue + } else { return false } - sys_sched_yield() - continue } } } diff --git a/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift b/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift index b1a24c8f09..64b74b1818 100644 --- a/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift +++ b/Sources/NIOCore/AsyncSequences/NIOAsyncWriter.swift @@ -1077,46 +1077,15 @@ extension NIOAsyncWriter { let delegate, let error ): - guard bufferedYieldIDs.contains(yieldID) else { - // We are already finished and still tried to write something - return .throwError(NIOAsyncWriterError.alreadyFinished()) - } - // This yield was buffered before we became finished so we still have to deliver it - self._state = .modifying - - if let index = cancelledYields.firstIndex(of: yieldID) { - // We already marked the yield as cancelled. We have to remove it and - // throw a CancellationError. - cancelledYields.remove(at: index) - - self._state = .writerFinished( - isWritable: isWritable, - inDelegateOutcall: inDelegateOutcall, - suspendedYields: suspendedYields, - cancelledYields: cancelledYields, - bufferedYieldIDs: bufferedYieldIDs, - delegate: delegate, - error: error - ) + if bufferedYieldIDs.contains(yieldID) { + // This yield was buffered before we became finished so we still have to deliver it + self._state = .modifying - return .throwError(CancellationError()) - } else { - // Yield hasn't been marked as cancelled. + if let index = cancelledYields.firstIndex(of: yieldID) { + // We already marked the yield as cancelled. We have to remove it and + // throw a CancellationError. + cancelledYields.remove(at: index) - switch (isWritable, inDelegateOutcall) { - case (true, false): - self._state = .writerFinished( - isWritable: isWritable, - inDelegateOutcall: true, // We are now making a call to the delegate - suspendedYields: suspendedYields, - cancelledYields: cancelledYields, - bufferedYieldIDs: bufferedYieldIDs, - delegate: delegate, - error: error - ) - - return .callDidYield(delegate) - case (true, true), (false, _): self._state = .writerFinished( isWritable: isWritable, inDelegateOutcall: inDelegateOutcall, @@ -1126,8 +1095,40 @@ extension NIOAsyncWriter { delegate: delegate, error: error ) - return .suspendTask + + return .throwError(CancellationError()) + } else { + // Yield hasn't been marked as cancelled. + + switch (isWritable, inDelegateOutcall) { + case (true, false): + self._state = .writerFinished( + isWritable: isWritable, + inDelegateOutcall: true, // We are now making a call to the delegate + suspendedYields: suspendedYields, + cancelledYields: cancelledYields, + bufferedYieldIDs: bufferedYieldIDs, + delegate: delegate, + error: error + ) + + return .callDidYield(delegate) + case (true, true), (false, _): + self._state = .writerFinished( + isWritable: isWritable, + inDelegateOutcall: inDelegateOutcall, + suspendedYields: suspendedYields, + cancelledYields: cancelledYields, + bufferedYieldIDs: bufferedYieldIDs, + delegate: delegate, + error: error + ) + return .suspendTask + } } + } else { + // We are already finished and still tried to write something + return .throwError(NIOAsyncWriterError.alreadyFinished()) } case .finished(let sinkError): @@ -1215,7 +1216,28 @@ extension NIOAsyncWriter { var suspendedYields, let delegate ): - guard let index = suspendedYields.firstIndex(where: { $0.yieldID == yieldID }) else { + if let index = suspendedYields.firstIndex(where: { $0.yieldID == yieldID }) { + self._state = .modifying + // We have a suspended yield for the id. We need to resume the continuation now. + + // Removing can be quite expensive if it produces a gap in the array. + // Since we are not expecting a lot of elements in this array it should be fine + // to just remove. If this turns out to be a performance pitfall, we can + // swap the elements before removing. So that we always remove the last element. + let suspendedYield = suspendedYields.remove(at: index) + + // We are keeping the elements that the yield produced. + self._state = .streaming( + isWritable: isWritable, + inDelegateOutcall: inDelegateOutcall, + cancelledYields: cancelledYields, + suspendedYields: suspendedYields, + delegate: delegate + ) + + return .resumeContinuationWithCancellationError(suspendedYield.continuation) + + } else { self._state = .modifying // There is no suspended yield. This can mean that we either already yielded // or that the call to `yield` is coming afterwards. We need to store @@ -1232,25 +1254,6 @@ extension NIOAsyncWriter { return .none } - self._state = .modifying - // We have a suspended yield for the id. We need to resume the continuation now. - - // Removing can be quite expensive if it produces a gap in the array. - // Since we are not expecting a lot of elements in this array it should be fine - // to just remove. If this turns out to be a performance pitfall, we can - // swap the elements before removing. So that we always remove the last element. - let suspendedYield = suspendedYields.remove(at: index) - - // We are keeping the elements that the yield produced. - self._state = .streaming( - isWritable: isWritable, - inDelegateOutcall: inDelegateOutcall, - cancelledYields: cancelledYields, - suspendedYields: suspendedYields, - delegate: delegate - ) - - return .resumeContinuationWithCancellationError(suspendedYield.continuation) case .writerFinished( let isWritable, @@ -1264,7 +1267,30 @@ extension NIOAsyncWriter { guard bufferedYieldIDs.contains(yieldID) else { return .none } - guard let index = suspendedYields.firstIndex(where: { $0.yieldID == yieldID }) else { + if let index = suspendedYields.firstIndex(where: { $0.yieldID == yieldID }) { + self._state = .modifying + // We have a suspended yield for the id. We need to resume the continuation now. + + // Removing can be quite expensive if it produces a gap in the array. + // Since we are not expecting a lot of elements in this array it should be fine + // to just remove. If this turns out to be a performance pitfall, we can + // swap the elements before removing. So that we always remove the last element. + let suspendedYield = suspendedYields.remove(at: index) + + // We are keeping the elements that the yield produced. + self._state = .writerFinished( + isWritable: isWritable, + inDelegateOutcall: inDelegateOutcall, + suspendedYields: suspendedYields, + cancelledYields: cancelledYields, + bufferedYieldIDs: bufferedYieldIDs, + delegate: delegate, + error: error + ) + + return .resumeContinuationWithCancellationError(suspendedYield.continuation) + + } else { self._state = .modifying // There is no suspended yield. This can mean that we either already yielded // or that the call to `yield` is coming afterwards. We need to store @@ -1283,27 +1309,6 @@ extension NIOAsyncWriter { return .none } - self._state = .modifying - // We have a suspended yield for the id. We need to resume the continuation now. - - // Removing can be quite expensive if it produces a gap in the array. - // Since we are not expecting a lot of elements in this array it should be fine - // to just remove. If this turns out to be a performance pitfall, we can - // swap the elements before removing. So that we always remove the last element. - let suspendedYield = suspendedYields.remove(at: index) - - // We are keeping the elements that the yield produced. - self._state = .writerFinished( - isWritable: isWritable, - inDelegateOutcall: inDelegateOutcall, - suspendedYields: suspendedYields, - cancelledYields: cancelledYields, - bufferedYieldIDs: bufferedYieldIDs, - delegate: delegate, - error: error - ) - - return .resumeContinuationWithCancellationError(suspendedYield.continuation) case .finished: // We are already finished and there is nothing to do @@ -1342,7 +1347,27 @@ extension NIOAsyncWriter { let delegate ): // We are currently streaming and the writer got finished. - guard suspendedYields.isEmpty else { + if suspendedYields.isEmpty { + if inDelegateOutcall { + // We are in an outcall already and have to buffer + // the didTerminate call. + self._state = .writerFinished( + isWritable: isWritable, + inDelegateOutcall: inDelegateOutcall, + suspendedYields: .init(), + cancelledYields: cancelledYields, + bufferedYieldIDs: .init(), + delegate: delegate, + error: error + ) + return .none + } else { + // We have no elements left and are not in an outcall so we + // can transition to finished directly + self._state = .finished(sinkError: nil) + return .callDidTerminate(delegate) + } + } else { // There are still suspended writer tasks which we need to deliver once we become writable again self._state = .writerFinished( isWritable: isWritable, @@ -1356,24 +1381,6 @@ extension NIOAsyncWriter { return .none } - guard inDelegateOutcall else { - // We have no elements left and are not in an outcall so we - // can transition to finished directly - self._state = .finished(sinkError: nil) - return .callDidTerminate(delegate) - } - // We are in an outcall already and have to buffer - // the didTerminate call. - self._state = .writerFinished( - isWritable: isWritable, - inDelegateOutcall: inDelegateOutcall, - suspendedYields: .init(), - cancelledYields: cancelledYields, - bufferedYieldIDs: .init(), - delegate: delegate, - error: error - ) - return .none case .writerFinished, .finished: // We are already finished and there is nothing to do @@ -1452,7 +1459,17 @@ extension NIOAsyncWriter { precondition(inDelegateOutcall, "We must be in a delegate outcall when we unbuffer events") // We have to resume the other suspended yields now. - guard suspendedYields.isEmpty else { + if suspendedYields.isEmpty { + // There are no other writer suspended writer tasks so we can just return + self._state = .streaming( + isWritable: isWritable, + inDelegateOutcall: false, + cancelledYields: cancelledYields, + suspendedYields: suspendedYields, + delegate: delegate + ) + return .none + } else { // We have to resume the other suspended yields now. self._state = .streaming( isWritable: isWritable, @@ -1463,15 +1480,6 @@ extension NIOAsyncWriter { ) return .resumeContinuations(suspendedYields) } - // There are no other writer suspended writer tasks so we can just return - self._state = .streaming( - isWritable: isWritable, - inDelegateOutcall: false, - cancelledYields: cancelledYields, - suspendedYields: suspendedYields, - delegate: delegate - ) - return .none case .writerFinished( let isWritable, @@ -1483,7 +1491,11 @@ extension NIOAsyncWriter { let error ): precondition(inDelegateOutcall, "We must be in a delegate outcall when we unbuffer events") - guard suspendedYields.isEmpty else { + if suspendedYields.isEmpty { + // We were the last writer task and can now call didTerminate + self._state = .finished(sinkError: nil) + return .callDidTerminate(delegate, error) + } else { // There are still other writer tasks that need to be resumed self._state = .modifying @@ -1499,9 +1511,6 @@ extension NIOAsyncWriter { return .resumeContinuations(suspendedYields) } - // We were the last writer task and can now call didTerminate - self._state = .finished(sinkError: nil) - return .callDidTerminate(delegate, error) case .finished: return .none diff --git a/Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift b/Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift index ca87c0086b..7b95ee86f2 100644 --- a/Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift +++ b/Sources/NIOCore/AsyncSequences/NIOThrowingAsyncSequenceProducer.swift @@ -1197,7 +1197,27 @@ extension NIOThrowingAsyncSequenceProducer { ): self._state = .modifying - guard let element = buffer.popFirst() else { + if let element = buffer.popFirst() { + // We have an element to fulfil the demand right away. + + let shouldProduceMore = backPressureStrategy.didConsume(bufferDepth: buffer.count) + + self._state = .streaming( + backPressureStrategy: backPressureStrategy, + buffer: buffer, + continuation: nil, + hasOutstandingDemand: shouldProduceMore, + iteratorInitialized: iteratorInitialized + ) + + if shouldProduceMore && !hasOutstandingDemand { + // We didn't have any demand but now we do, so we need to inform the delegate. + return .returnElementAndCallProduceMore(element) + } else { + // We don't have any new demand, so we can just return the element. + return .returnElement(element) + } + } else { // There is nothing in the buffer to fulfil the demand so we need to suspend. // We are not interacting with the back-pressure strategy here because // we are doing this inside `next(:)` @@ -1211,42 +1231,25 @@ extension NIOThrowingAsyncSequenceProducer { return .suspendTask } - // We have an element to fulfil the demand right away. - - let shouldProduceMore = backPressureStrategy.didConsume(bufferDepth: buffer.count) - - self._state = .streaming( - backPressureStrategy: backPressureStrategy, - buffer: buffer, - continuation: nil, - hasOutstandingDemand: shouldProduceMore, - iteratorInitialized: iteratorInitialized - ) - - guard shouldProduceMore && !hasOutstandingDemand else { - // We don't have any new demand, so we can just return the element. - return .returnElement(element) - } - // We didn't have any demand but now we do, so we need to inform the delegate. - return .returnElementAndCallProduceMore(element) case .sourceFinished(var buffer, let iteratorInitialized, let failure): self._state = .modifying // Check if we have an element left in the buffer and return it - guard let element = buffer.popFirst() else { + if let element = buffer.popFirst() { + self._state = .sourceFinished( + buffer: buffer, + iteratorInitialized: iteratorInitialized, + failure: failure + ) + + return .returnElement(element) + } else { // We are returning the queued failure now and can transition to finished self._state = .finished(iteratorInitialized: iteratorInitialized) return .returnFailureAndCallDidTerminate(failure) } - self._state = .sourceFinished( - buffer: buffer, - iteratorInitialized: iteratorInitialized, - failure: failure - ) - - return .returnElement(element) case .cancelled(let iteratorInitialized): self._state = .finished(iteratorInitialized: iteratorInitialized) @@ -1296,10 +1299,11 @@ extension NIOThrowingAsyncSequenceProducer { iteratorInitialized: iteratorInitialized ) - guard shouldProduceMore && !hasOutstandingDemand else { + if shouldProduceMore && !hasOutstandingDemand { + return .callProduceMore + } else { return .none } - return .callProduceMore case .streaming(_, _, .some(_), _, _), .sourceFinished, .finished, .cancelled: preconditionFailure("This should have already been handled by `next()`") diff --git a/Sources/NIOCore/ByteBuffer-aux.swift b/Sources/NIOCore/ByteBuffer-aux.swift index 22c9efa094..fcfa74ce3b 100644 --- a/Sources/NIOCore/ByteBuffer-aux.swift +++ b/Sources/NIOCore/ByteBuffer-aux.swift @@ -119,14 +119,13 @@ extension ByteBuffer { @inlinable mutating func _setStringSlowpath(_ string: String, at index: Int) -> Int { // slow path, let's try to force the string to be native - guard - let written = (string + "").utf8.withContiguousStorageIfAvailable({ utf8Bytes in - self.setBytes(utf8Bytes, at: index) - }) - else { + if let written = (string + "").utf8.withContiguousStorageIfAvailable({ utf8Bytes in + self.setBytes(utf8Bytes, at: index) + }) { + return written + } else { return self.setBytes(string.utf8, at: index) } - return written } /// Write `string` into this `ByteBuffer` at `index` using UTF-8 encoding. Does not move the writer index. @@ -141,15 +140,14 @@ extension ByteBuffer { // Do not implement setString via setSubstring. Before Swift version 5.3, // Substring.UTF8View did not implement withContiguousStorageIfAvailable // and therefore had no fast access to the backing storage. - guard - let written = string.utf8.withContiguousStorageIfAvailable({ utf8Bytes in - self.setBytes(utf8Bytes, at: index) - }) - else { + if let written = string.utf8.withContiguousStorageIfAvailable({ utf8Bytes in + self.setBytes(utf8Bytes, at: index) + }) { + // fast path, directly available + return written + } else { return self._setStringSlowpath(string, at: index) } - // fast path, directly available - return written } /// Write `string` null terminated into this `ByteBuffer` at `index` using UTF-8 encoding. Does not move the writer index. @@ -264,16 +262,15 @@ extension ByteBuffer { public mutating func setSubstring(_ substring: Substring, at index: Int) -> Int { // Substring.UTF8View implements withContiguousStorageIfAvailable starting with // Swift version 5.3. - guard - let written = substring.utf8.withContiguousStorageIfAvailable({ utf8Bytes in - self.setBytes(utf8Bytes, at: index) - }) - else { + if let written = substring.utf8.withContiguousStorageIfAvailable({ utf8Bytes in + self.setBytes(utf8Bytes, at: index) + }) { + // fast path, directly available + return written + } else { // slow path, convert to string return self.setString(String(substring), at: index) } - // fast path, directly available - return written } // MARK: DispatchData APIs @@ -847,12 +844,13 @@ extension Optional where Wrapped == ByteBuffer { @discardableResult @inlinable public mutating func setOrWriteBuffer(_ buffer: inout ByteBuffer) -> Int { - guard self == nil else { + if self == nil { + let readableBytes = buffer.readableBytes + self = buffer + buffer.moveReaderIndex(to: buffer.writerIndex) + return readableBytes + } else { return self!.writeBuffer(&buffer) } - let readableBytes = buffer.readableBytes - self = buffer - buffer.moveReaderIndex(to: buffer.writerIndex) - return readableBytes } } diff --git a/Sources/NIOCore/ByteBuffer-core.swift b/Sources/NIOCore/ByteBuffer-core.swift index e782c77184..8027fcc637 100644 --- a/Sources/NIOCore/ByteBuffer-core.swift +++ b/Sources/NIOCore/ByteBuffer-core.swift @@ -537,15 +537,14 @@ public struct ByteBuffer { @inlinable mutating func _setBytes(_ bytes: Bytes, at index: _Index) -> _Capacity where Bytes.Element == UInt8 { - guard - let written = bytes.withContiguousStorageIfAvailable({ bytes in - self._setBytes(UnsafeRawBufferPointer(bytes), at: index) - }) - else { + if let written = bytes.withContiguousStorageIfAvailable({ bytes in + self._setBytes(UnsafeRawBufferPointer(bytes), at: index) + }) { + // fast path, we've got access to the contiguous bytes + return written + } else { return self._setSlowPath(bytes: bytes, at: index) } - // fast path, we've got access to the contiguous bytes - return written } // MARK: Public Core API @@ -1191,10 +1190,11 @@ extension ByteBuffer { /// - returns: The return value of `body`. @inlinable public mutating func modifyIfUniquelyOwned(_ body: (inout ByteBuffer) throws -> T) rethrows -> T? { - guard isKnownUniquelyReferenced(&self._storage) else { + if isKnownUniquelyReferenced(&self._storage) { + return try body(&self) + } else { return nil } - return try body(&self) } } diff --git a/Sources/NIOCore/ByteBuffer-hexdump.swift b/Sources/NIOCore/ByteBuffer-hexdump.swift index 6e6bcd7d13..fa5dde44e8 100644 --- a/Sources/NIOCore/ByteBuffer-hexdump.swift +++ b/Sources/NIOCore/ByteBuffer-hexdump.swift @@ -248,16 +248,18 @@ extension ByteBuffer { public func hexDump(format: HexDumpFormat) -> String { switch format.value { case .plain(let maxBytes): - guard let maxBytes = maxBytes else { + if let maxBytes = maxBytes { + return self.hexDumpPlain(maxBytes: maxBytes) + } else { return self.hexDumpPlain() } - return self.hexDumpPlain(maxBytes: maxBytes) case .detailed(let maxBytes): - guard let maxBytes = maxBytes else { + if let maxBytes = maxBytes { + return self.hexDumpDetailed(maxBytes: maxBytes) + } else { return self.hexdumpDetailed() } - return self.hexDumpDetailed(maxBytes: maxBytes) } } } diff --git a/Sources/NIOCore/ChannelOption.swift b/Sources/NIOCore/ChannelOption.swift index d1029d8df5..a4e0746a04 100644 --- a/Sources/NIOCore/ChannelOption.swift +++ b/Sources/NIOCore/ChannelOption.swift @@ -392,11 +392,12 @@ extension ChannelOptions { var hasSet = false self._storage = self._storage.map { currentKeyAndValue in let (currentKey, _) = currentKeyAndValue - guard let currentKey = currentKey as? Option, currentKey == newKey else { + if let currentKey = currentKey as? Option, currentKey == newKey { + hasSet = true + return (currentKey, (newValue, applier)) + } else { return currentKeyAndValue } - hasSet = true - return (currentKey, (newValue, applier)) } if !hasSet { self._storage.append((newKey, (newValue, applier))) diff --git a/Sources/NIOCore/ChannelPipeline.swift b/Sources/NIOCore/ChannelPipeline.swift index 508b9a2d7c..aa0ab19275 100644 --- a/Sources/NIOCore/ChannelPipeline.swift +++ b/Sources/NIOCore/ChannelPipeline.swift @@ -570,10 +570,11 @@ public final class ChannelPipeline: ChannelInvoker { internal func _contextSync(_ body: (ChannelHandlerContext) -> Bool) -> Result { self.eventLoop.assertInEventLoop() - guard let context = self.contextForPredicate0(body) else { + if let context = self.contextForPredicate0(body) { + return .success(context) + } else { return .failure(ChannelPipelineError.notFound) } - return .success(context) } /// Returns a `ChannelHandlerContext` which matches. diff --git a/Sources/NIOCore/CircularBuffer.swift b/Sources/NIOCore/CircularBuffer.swift index 7a07a505b0..360b3dfb0d 100644 --- a/Sources/NIOCore/CircularBuffer.swift +++ b/Sources/NIOCore/CircularBuffer.swift @@ -456,10 +456,11 @@ extension CircularBuffer { /// Returns the number of element in the ring. @inlinable public var count: Int { - guard self.tailBackingIndex >= self.headBackingIndex else { + if self.tailBackingIndex >= self.headBackingIndex { + return self.tailBackingIndex &- self.headBackingIndex + } else { return self._buffer.count &- (self.headBackingIndex &- self.tailBackingIndex) } - return self.tailBackingIndex &- self.headBackingIndex } /// The total number of elements that the ring can contain without allocating new storage. @@ -537,10 +538,11 @@ extension CircularBuffer: RangeReplaceableCollection { /// - Complexity: O(1) @inlinable public mutating func popFirst() -> Element? { - guard count > 0 else { + if count > 0 { + return self.removeFirst() + } else { return nil } - return self.removeFirst() } /// Removes and returns the last element of the `CircularBuffer`. @@ -555,10 +557,11 @@ extension CircularBuffer: RangeReplaceableCollection { /// - Complexity: O(1) @inlinable public mutating func popLast() -> Element? { - guard count > 0 else { + if count > 0 { + return self.removeLast() + } else { return nil } - return self.removeLast() } /// Removes the specified number of elements from the end of the diff --git a/Sources/NIOCore/Codec.swift b/Sources/NIOCore/Codec.swift index 452224113c..6b9d59c4f7 100644 --- a/Sources/NIOCore/Codec.swift +++ b/Sources/NIOCore/Codec.swift @@ -304,11 +304,12 @@ extension B2MDBuffer { var buffer = self.buffers.removeFirst() buffer.writeBuffers(self.buffers) self.buffers.removeAll(keepingCapacity: self.buffers.capacity < 16) // don't grow too much - guard buffer.readableBytes > 0 || allowEmptyBuffer else { + if buffer.readableBytes > 0 || allowEmptyBuffer { + self.state = .processingInProgress + return .available(buffer) + } else { return .nothingAvailable } - self.state = .processingInProgress - return .available(buffer) case .ready: assert(self.buffers.isEmpty) if allowEmptyBuffer { @@ -617,11 +618,12 @@ extension ByteToMessageHandler { self.tryDecodeWrites() continue case .didProcess(.needMoreData): - guard self.queuedWrites.count > 0 else { + if self.queuedWrites.count > 0 { + self.tryDecodeWrites() + continue // we might have received more, so let's spin once more + } else { return .didProcess(.needMoreData) } - self.tryDecodeWrites() - continue // we might have received more, so let's spin once more case .cannotProcessReentrantly: return .cannotProcessReentrantly } diff --git a/Sources/NIOCore/EventLoop.swift b/Sources/NIOCore/EventLoop.swift index c451d61360..1f38c73a31 100644 --- a/Sources/NIOCore/EventLoop.swift +++ b/Sources/NIOCore/EventLoop.swift @@ -502,10 +502,11 @@ public struct TimeAmount: Hashable, Sendable { @inlinable static func _cappedNanoseconds(amount: Int64, multiplier: Int64) -> Int64 { let nanosecondsMultiplication = amount.multipliedReportingOverflow(by: multiplier) - guard nanosecondsMultiplication.overflow else { + if nanosecondsMultiplication.overflow { + return amount >= 0 ? .max : .min + } else { return nanosecondsMultiplication.partialValue } - return amount >= 0 ? .max : .min } } @@ -851,11 +852,12 @@ extension EventLoop { /// - returns: a succeeded `EventLoopFuture`. @inlinable public func makeSucceededFuture(_ value: Success) -> EventLoopFuture { - guard Success.self == Void.self else { + if Success.self == Void.self { + // The as! will always succeed because we previously checked that Success.self == Void.self. + return self.makeSucceededVoidFuture() as! EventLoopFuture + } else { return EventLoopFuture(eventLoop: self, value: value) } - // The as! will always succeed because we previously checked that Success.self == Void.self. - return self.makeSucceededVoidFuture() as! EventLoopFuture } /// Creates and returns a new `EventLoopFuture` that is marked as succeeded or failed with the value held by `result`. diff --git a/Sources/NIOCore/EventLoopFuture+WithEventLoop.swift b/Sources/NIOCore/EventLoopFuture+WithEventLoop.swift index 906821892d..bf76a0e97e 100644 --- a/Sources/NIOCore/EventLoopFuture+WithEventLoop.swift +++ b/Sources/NIOCore/EventLoopFuture+WithEventLoop.swift @@ -49,13 +49,14 @@ extension EventLoopFuture { switch self._value! { case .success(let t): let futureU = callback(t, eventLoop) - guard futureU.eventLoop.inEventLoop else { + if futureU.eventLoop.inEventLoop { + return futureU._addCallback { + next._setValue(value: futureU._value!) + } + } else { futureU.cascade(to: next) return CallbackList() } - return futureU._addCallback { - next._setValue(value: futureU._value!) - } case .failure(let error): return next._setValue(value: .failure(error)) } @@ -86,13 +87,14 @@ extension EventLoopFuture { return next._setValue(value: .success(t)) case .failure(let e): let t = callback(e, eventLoop) - guard t.eventLoop.inEventLoop else { + if t.eventLoop.inEventLoop { + return t._addCallback { + next._setValue(value: t._value!) + } + } else { t.cascade(to: next) return CallbackList() } - return t._addCallback { - next._setValue(value: t._value!) - } } } return next.futureResult @@ -134,13 +136,14 @@ extension EventLoopFuture { return body } - guard self.eventLoop.inEventLoop else { + if self.eventLoop.inEventLoop { + return fold0(eventLoop: self.eventLoop) + } else { let promise = self.eventLoop.makePromise(of: Value.self) self.eventLoop.execute { [eventLoop = self.eventLoop] in fold0(eventLoop: eventLoop).cascade(to: promise) } return promise.futureResult } - return fold0(eventLoop: self.eventLoop) } } diff --git a/Sources/NIOCore/EventLoopFuture.swift b/Sources/NIOCore/EventLoopFuture.swift index baf0602094..65b18c048c 100644 --- a/Sources/NIOCore/EventLoopFuture.swift +++ b/Sources/NIOCore/EventLoopFuture.swift @@ -486,13 +486,14 @@ extension EventLoopFuture { switch self._value! { case .success(let t): let futureU = callback(t) - guard futureU.eventLoop.inEventLoop else { + if futureU.eventLoop.inEventLoop { + return futureU._addCallback { + next._setValue(value: futureU._value!) + } + } else { futureU.cascade(to: next) return CallbackList() } - return futureU._addCallback { - next._setValue(value: futureU._value!) - } case .failure(let error): return next._setValue(value: .failure(error)) } @@ -620,15 +621,16 @@ extension EventLoopFuture { @inlinable func _map(_ callback: @escaping MapCallback) -> EventLoopFuture { - guard NewValue.self == Value.self && NewValue.self == Void.self else { + if NewValue.self == Value.self && NewValue.self == Void.self { + self.whenSuccess(callback as! @Sendable (Value) -> Void) + return self as! EventLoopFuture + } else { let next = EventLoopPromise.makeUnleakablePromise(eventLoop: self.eventLoop) self._whenComplete { next._setValue(value: self._value!.map(callback)) } return next.futureResult } - self.whenSuccess(callback as! @Sendable (Value) -> Void) - return self as! EventLoopFuture } /// When the current `EventLoopFuture` is in an error state, run the provided callback, which @@ -660,13 +662,14 @@ extension EventLoopFuture { return next._setValue(value: .success(t)) case .failure(let e): let t = callback(e) - guard t.eventLoop.inEventLoop else { + if t.eventLoop.inEventLoop { + return t._addCallback { + next._setValue(value: t._value!) + } + } else { t.cascade(to: next) return CallbackList() } - return t._addCallback { - next._setValue(value: t._value!) - } } } return next.futureResult @@ -1084,14 +1087,15 @@ extension EventLoopFuture { return body } - guard self.eventLoop.inEventLoop else { + if self.eventLoop.inEventLoop { + return fold0() + } else { let promise = self.eventLoop.makePromise(of: Value.self) self.eventLoop.execute { fold0().cascade(to: promise) } return promise.futureResult } - return fold0() } } diff --git a/Sources/NIOCore/GlobalSingletons.swift b/Sources/NIOCore/GlobalSingletons.swift index 5ca11af85c..384dfc6d71 100644 --- a/Sources/NIOCore/GlobalSingletons.swift +++ b/Sources/NIOCore/GlobalSingletons.swift @@ -89,15 +89,16 @@ extension NIOSingletons { desired: 1, ordering: .relaxed ) - guard exchanged else { + if exchanged { + // Never been set, we're committing to the default (enabled). + assert(original == 0) + return true + } else { // This has been set before, 1: enabled; -1 disabled. assert(original != 0) assert(original == -1 || original == 1) return original > 0 } - // Never been set, we're committing to the default (enabled). - assert(original == 0) - return true } set { diff --git a/Sources/NIOCore/IO.swift b/Sources/NIOCore/IO.swift index d49de00c1e..49eb52e4d4 100644 --- a/Sources/NIOCore/IO.swift +++ b/Sources/NIOCore/IO.swift @@ -121,10 +121,11 @@ public struct IOError: Swift.Error { /// - reason: what failed /// - returns: the constructed reason. private func reasonForError(errnoCode: CInt, reason: String) -> String { - guard let errorDescC = strerror(errnoCode) else { + if let errorDescC = strerror(errnoCode) { + return "\(reason): \(String(cString: errorDescC)) (errno: \(errnoCode))" + } else { return "\(reason): Broken strerror, unknown error: \(errnoCode)" } - return "\(reason): \(String(cString: errorDescC)) (errno: \(errnoCode))" } #if os(Windows) diff --git a/Sources/NIOCore/MarkedCircularBuffer.swift b/Sources/NIOCore/MarkedCircularBuffer.swift index baf7d4e541..1881c824cb 100644 --- a/Sources/NIOCore/MarkedCircularBuffer.swift +++ b/Sources/NIOCore/MarkedCircularBuffer.swift @@ -98,20 +98,22 @@ public struct MarkedCircularBuffer: CustomStringConvertible { public func isMarked(index: Index) -> Bool { assert(index >= self.startIndex, "index must not be negative") precondition(index < self.endIndex, "index \(index) out of range (0..<\(self._buffer.count))") - guard let markedIndexOffset = self._markedIndexOffset else { + if let markedIndexOffset = self._markedIndexOffset { + return self.index(self.startIndex, offsetBy: markedIndexOffset) == index + } else { return false } - return self.index(self.startIndex, offsetBy: markedIndexOffset) == index } /// Returns the index of the marked element. @inlinable public var markedElementIndex: Index? { - guard let markedIndexOffset = self._markedIndexOffset else { + if let markedIndexOffset = self._markedIndexOffset { + assert(markedIndexOffset >= 0) + return self.index(self.startIndex, offsetBy: markedIndexOffset) + } else { return nil } - assert(markedIndexOffset >= 0) - return self.index(self.startIndex, offsetBy: markedIndexOffset) } /// Returns the marked element. diff --git a/Sources/NIOCore/NIOAny.swift b/Sources/NIOCore/NIOAny.swift index 8b751b749d..1f687b134b 100644 --- a/Sources/NIOCore/NIOAny.swift +++ b/Sources/NIOCore/NIOAny.swift @@ -83,10 +83,11 @@ public struct NIOAny { /// - returns: The wrapped `ByteBuffer` or `nil` if the wrapped message is not a `ByteBuffer`. @inlinable func tryAsByteBuffer() -> ByteBuffer? { - guard case .ioData(.byteBuffer(let bb)) = self._storage else { + if case .ioData(.byteBuffer(let bb)) = self._storage { + return bb + } else { return nil } - return bb } /// Force unwrapping the wrapped message as `ByteBuffer`. @@ -108,10 +109,11 @@ public struct NIOAny { /// - returns: The wrapped `IOData` or `nil` if the wrapped message is not a `IOData`. @inlinable func tryAsIOData() -> IOData? { - guard case .ioData(let data) = self._storage else { + if case .ioData(let data) = self._storage { + return data + } else { return nil } - return data } /// Force unwrapping the wrapped message as `IOData`. @@ -133,10 +135,11 @@ public struct NIOAny { /// - returns: The wrapped `FileRegion` or `nil` if the wrapped message is not a `FileRegion`. @inlinable func tryAsFileRegion() -> FileRegion? { - guard case .ioData(.fileRegion(let f)) = self._storage else { + if case .ioData(.fileRegion(let f)) = self._storage { + return f + } else { return nil } - return f } /// Force unwrapping the wrapped message as `FileRegion`. @@ -158,10 +161,11 @@ public struct NIOAny { /// - returns: The wrapped `AddressedEnvelope` or `nil` if the wrapped message is not an `AddressedEnvelope`. @inlinable func tryAsByteEnvelope() -> AddressedEnvelope? { - guard case .bufferEnvelope(let e) = self._storage else { + if case .bufferEnvelope(let e) = self._storage { + return e + } else { return nil } - return e } /// Force unwrapping the wrapped message as `AddressedEnvelope`. diff --git a/Sources/NIOCore/SingleStepByteToMessageDecoder.swift b/Sources/NIOCore/SingleStepByteToMessageDecoder.swift index bae7cb9a39..1fed8965ac 100644 --- a/Sources/NIOCore/SingleStepByteToMessageDecoder.swift +++ b/Sources/NIOCore/SingleStepByteToMessageDecoder.swift @@ -53,11 +53,12 @@ public protocol NIOSingleStepByteToMessageDecoder: ByteToMessageDecoder { // MARK: NIOSingleStepByteToMessageDecoder: ByteToMessageDecoder extension NIOSingleStepByteToMessageDecoder { public mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - guard let message = try self.decode(buffer: &buffer) else { + if let message = try self.decode(buffer: &buffer) { + context.fireChannelRead(Self.wrapInboundOut(message)) + return .continue + } else { return .needMoreData } - context.fireChannelRead(Self.wrapInboundOut(message)) - return .continue } public mutating func decodeLast( @@ -65,11 +66,12 @@ extension NIOSingleStepByteToMessageDecoder { buffer: inout ByteBuffer, seenEOF: Bool ) throws -> DecodingState { - guard let message = try self.decodeLast(buffer: &buffer, seenEOF: seenEOF) else { + if let message = try self.decodeLast(buffer: &buffer, seenEOF: seenEOF) { + context.fireChannelRead(Self.wrapInboundOut(message)) + return .continue + } else { return .needMoreData } - context.fireChannelRead(Self.wrapInboundOut(message)) - return .continue } } @@ -255,10 +257,11 @@ public final class NIOSingleStepByteToMessageProcessor Decoder.InboundOut? { - guard decodeMode == .normal else { + if decodeMode == .normal { + return try self.decoder.decode(buffer: &buffer) + } else { return try self.decoder.decodeLast(buffer: &buffer, seenEOF: seenEOF) } - return try self.decoder.decode(buffer: &buffer) } while let message = try self._withNonCoWBuffer(decodeOnce) { diff --git a/Sources/NIOCore/SocketAddresses.swift b/Sources/NIOCore/SocketAddresses.swift index 98710662e9..0c52ba7a8e 100644 --- a/Sources/NIOCore/SocketAddresses.swift +++ b/Sources/NIOCore/SocketAddresses.swift @@ -496,19 +496,20 @@ public enum SocketAddress: CustomStringConvertible, Sendable { } } - guard let info = info, let addrPointer = info.pointee.ai_addr else { + if let info = info, let addrPointer = info.pointee.ai_addr { + let addressBytes = UnsafeRawPointer(addrPointer) + switch NIOBSDSocket.AddressFamily(rawValue: info.pointee.ai_family) { + case .inet: + return .v4(.init(address: addressBytes.load(as: sockaddr_in.self), host: host)) + case .inet6: + return .v6(.init(address: addressBytes.load(as: sockaddr_in6.self), host: host)) + default: + throw SocketAddressError.unsupported + } + } else { // this is odd, getaddrinfo returned NULL throw SocketAddressError.unsupported } - let addressBytes = UnsafeRawPointer(addrPointer) - switch NIOBSDSocket.AddressFamily(rawValue: info.pointee.ai_family) { - case .inet: - return .v4(.init(address: addressBytes.load(as: sockaddr_in.self), host: host)) - case .inet6: - return .v6(.init(address: addressBytes.load(as: sockaddr_in6.self), host: host)) - default: - throw SocketAddressError.unsupported - } #endif } } diff --git a/Sources/NIOCrashTester/OutputGrepper.swift b/Sources/NIOCrashTester/OutputGrepper.swift index 751245fba6..7802fe83b9 100644 --- a/Sources/NIOCrashTester/OutputGrepper.swift +++ b/Sources/NIOCrashTester/OutputGrepper.swift @@ -92,11 +92,12 @@ private struct NewlineFramer: ByteToMessageDecoder { typealias InboundOut = String func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - guard let firstNewline = buffer.readableBytesView.firstIndex(of: UInt8(ascii: "\n")) else { + if let firstNewline = buffer.readableBytesView.firstIndex(of: UInt8(ascii: "\n")) { + let length = firstNewline - buffer.readerIndex + 1 + context.fireChannelRead(Self.wrapInboundOut(String(buffer.readString(length: length)!.dropLast()))) + return .continue + } else { return .needMoreData } - let length = firstNewline - buffer.readerIndex + 1 - context.fireChannelRead(Self.wrapInboundOut(String(buffer.readString(length: length)!.dropLast()))) - return .continue } } diff --git a/Sources/NIOCrashTester/main.swift b/Sources/NIOCrashTester/main.swift index 358a2a8dd8..6234947aea 100644 --- a/Sources/NIOCrashTester/main.swift +++ b/Sources/NIOCrashTester/main.swift @@ -33,10 +33,11 @@ struct CrashTest { extension Process { var binaryPath: String? { get { - guard #available(macOS 10.13, *) else { + if #available(macOS 10.13, *) { + return self.executableURL?.path + } else { return self.launchPath } - return self.executableURL?.path } set { if #available(macOS 10.13, *) { @@ -113,10 +114,11 @@ func main() throws { } let output = try result.get() - guard output.range(of: regex, options: .regularExpression) != nil else { + if output.range(of: regex, options: .regularExpression) != nil { + return .crashedAsExpected + } else { return .regexDidNotMatch(regex: regex, output: output) } - return .crashedAsExpected } func usage() { diff --git a/Sources/NIOEmbedded/AsyncTestingChannel.swift b/Sources/NIOEmbedded/AsyncTestingChannel.swift index d6fec89319..11900d8353 100644 --- a/Sources/NIOEmbedded/AsyncTestingChannel.swift +++ b/Sources/NIOEmbedded/AsyncTestingChannel.swift @@ -100,10 +100,11 @@ public final class NIOAsyncTestingChannel: Channel { /// `true` if the ``NIOAsyncTestingChannel`` was `clean` on ``NIOAsyncTestingChannel/finish()``, ie. there is no unconsumed inbound, outbound, or /// pending outbound data left on the `Channel`. public var isClean: Bool { - guard case .clean = self else { + if case .clean = self { + return true + } else { return false } - return true } /// `true` if the ``NIOAsyncTestingChannel`` if there was unconsumed inbound, outbound, or pending outbound data left @@ -130,10 +131,11 @@ public final class NIOAsyncTestingChannel: Channel { /// Returns `true` is the buffer was empty. public var isEmpty: Bool { - guard case .empty = self else { + if case .empty = self { + return true + } else { return false } - return true } /// Returns `true` if the buffer was non-empty. @@ -333,14 +335,15 @@ public final class NIOAsyncTestingChannel: Channel { // This can never actually throw. return try! await self.testingEventLoop.executeInContext { let c = self.channelcore! - guard c.outboundBuffer.isEmpty && c.inboundBuffer.isEmpty && c.pendingOutboundBuffer.isEmpty else { + if c.outboundBuffer.isEmpty && c.inboundBuffer.isEmpty && c.pendingOutboundBuffer.isEmpty { + return .clean + } else { return .leftOvers( inbound: c.inboundBuffer, outbound: c.outboundBuffer, pendingOutbound: c.pendingOutboundBuffer.map { $0.0 } ) } - return .clean } } @@ -541,11 +544,12 @@ public final class NIOAsyncTestingChannel: Channel { /// - see: `Channel.setOption` @inlinable public func setOption(_ option: Option, value: Option.Value) -> EventLoopFuture { - guard self.eventLoop.inEventLoop else { + if self.eventLoop.inEventLoop { + self.setOptionSync(option, value: value) + return self.eventLoop.makeSucceededVoidFuture() + } else { return self.eventLoop.submit { self.setOptionSync(option, value: value) } } - self.setOptionSync(option, value: value) - return self.eventLoop.makeSucceededVoidFuture() } @inlinable @@ -561,10 +565,11 @@ public final class NIOAsyncTestingChannel: Channel { /// - see: `Channel.getOption` @inlinable public func getOption(_ option: Option) -> EventLoopFuture { - guard self.eventLoop.inEventLoop else { + if self.eventLoop.inEventLoop { + return self.eventLoop.makeSucceededFuture(self.getOptionSync(option)) + } else { return self.eventLoop.submit { self.getOptionSync(option) } } - return self.eventLoop.makeSucceededFuture(self.getOptionSync(option)) } @inlinable diff --git a/Sources/NIOEmbedded/Embedded.swift b/Sources/NIOEmbedded/Embedded.swift index a257c3977c..8f808e3f02 100644 --- a/Sources/NIOEmbedded/Embedded.swift +++ b/Sources/NIOEmbedded/Embedded.swift @@ -47,10 +47,11 @@ internal struct EmbeddedScheduledTask { extension EmbeddedScheduledTask: Comparable { static func < (lhs: EmbeddedScheduledTask, rhs: EmbeddedScheduledTask) -> Bool { - guard lhs.readyTime == rhs.readyTime else { + if lhs.readyTime == rhs.readyTime { + return lhs.insertOrder < rhs.insertOrder + } else { return lhs.readyTime < rhs.readyTime } - return lhs.insertOrder < rhs.insertOrder } static func == (lhs: EmbeddedScheduledTask, rhs: EmbeddedScheduledTask) -> Bool { @@ -346,19 +347,21 @@ class EmbeddedChannelCore: ChannelCore { @usableFromInline func localAddress0() throws -> SocketAddress { self.eventLoop.preconditionInEventLoop() - guard let localAddress = self.localAddress else { + if let localAddress = self.localAddress { + return localAddress + } else { throw ChannelError.operationUnsupported } - return localAddress } @usableFromInline func remoteAddress0() throws -> SocketAddress { self.eventLoop.preconditionInEventLoop() - guard let remoteAddress = self.remoteAddress else { + if let remoteAddress = self.remoteAddress { + return remoteAddress + } else { throw ChannelError.operationUnsupported } - return remoteAddress } @usableFromInline @@ -518,10 +521,11 @@ public final class EmbeddedChannel: Channel { /// `true` if the `EmbeddedChannel` was `clean` on `finish`, ie. there is no unconsumed inbound, outbound, or /// pending outbound data left on the `Channel`. public var isClean: Bool { - guard case .clean = self else { + if case .clean = self { + return true + } else { return false } - return true } /// `true` if the `EmbeddedChannel` if there was unconsumed inbound, outbound, or pending outbound data left @@ -547,10 +551,11 @@ public final class EmbeddedChannel: Channel { /// Returns `true` is the buffer was empty. public var isEmpty: Bool { - guard case .empty = self else { + if case .empty = self { + return true + } else { return false } - return true } /// Returns `true` if the buffer was non-empty. @@ -639,14 +644,15 @@ public final class EmbeddedChannel: Channel { self.embeddedEventLoop.run() try throwIfErrorCaught() let c = self.channelcore - guard c.outboundBuffer.isEmpty && c.inboundBuffer.isEmpty && c.pendingOutboundBuffer.isEmpty else { + if c.outboundBuffer.isEmpty && c.inboundBuffer.isEmpty && c.pendingOutboundBuffer.isEmpty { + return .clean + } else { return .leftOvers( inbound: Array(c.inboundBuffer), outbound: Array(c.outboundBuffer), pendingOutbound: c.pendingOutboundBuffer.map { $0.0 } ) } - return .clean } /// Synchronously closes the `EmbeddedChannel`. diff --git a/Sources/NIOFileSystem/BufferedReader.swift b/Sources/NIOFileSystem/BufferedReader.swift index cbe00da844..d66aa5d95e 100644 --- a/Sources/NIOFileSystem/BufferedReader.swift +++ b/Sources/NIOFileSystem/BufferedReader.swift @@ -79,7 +79,9 @@ public struct BufferedReader { let byteCount = Int(count.bytes) guard byteCount > 0 else { return ByteBuffer() } - guard let bytes = self.buffer.readSlice(length: byteCount) else { + if let bytes = self.buffer.readSlice(length: byteCount) { + return bytes + } else { // Not enough bytes: read enough for the caller and to fill the buffer back to capacity. var buffer = self.buffer self.buffer = ByteBuffer() @@ -102,7 +104,6 @@ public struct BufferedReader { return buffer } - return bytes } /// Reads from the current position in the file until `predicate` returns `false` and returns diff --git a/Sources/NIOFileSystem/FileChunks.swift b/Sources/NIOFileSystem/FileChunks.swift index 2d91f05563..b42d76e18d 100644 --- a/Sources/NIOFileSystem/FileChunks.swift +++ b/Sources/NIOFileSystem/FileChunks.swift @@ -164,23 +164,25 @@ private struct FileChunkProducer: Sendable { try self.state.withLockedValue { state in state.produceMore() }.flatMap { - guard let (descriptor, range) = $0 else { + if let (descriptor, range) = $0 { + if let range { + let remainingBytes = range.upperBound - range.lowerBound + let clampedLength = min(self.length, remainingBytes) + return descriptor.readChunk( + fromAbsoluteOffset: range.lowerBound, + length: clampedLength + ).mapError { error in + .read(usingSyscall: .pread, error: error, path: self.path, location: .here()) + } + } else { + return descriptor.readChunk(length: self.length).mapError { error in + .read(usingSyscall: .read, error: error, path: self.path, location: .here()) + } + } + } else { // nil means done: return empty to indicate the stream should finish. return .success(ByteBuffer()) } - guard let range else { - return descriptor.readChunk(length: self.length).mapError { error in - .read(usingSyscall: .read, error: error, path: self.path, location: .here()) - } - } - let remainingBytes = range.upperBound - range.lowerBound - let clampedLength = min(self.length, remainingBytes) - return descriptor.readChunk( - fromAbsoluteOffset: range.lowerBound, - length: clampedLength - ).mapError { error in - .read(usingSyscall: .pread, error: error, path: self.path, location: .here()) - } }.get() } @@ -289,7 +291,9 @@ private struct ProducerState: Sendable { mutating func produceMore() -> Result<(FileDescriptor, Range?)?, FileSystemError> { switch self.state { case let .producing(state): - guard let descriptor = state.handle.descriptorIfAvailable() else { + if let descriptor = state.handle.descriptorIfAvailable() { + return .success((descriptor, state.range)) + } else { let error = FileSystemError( code: .closed, message: "Cannot read from closed file ('\(state.handle.path)').", @@ -298,7 +302,6 @@ private struct ProducerState: Sendable { ) return .failure(error) } - return .success((descriptor, state.range)) case .done: return .success(nil) } diff --git a/Sources/NIOFileSystem/FileHandleProtocol.swift b/Sources/NIOFileSystem/FileHandleProtocol.swift index e1696c8562..d58e33065a 100644 --- a/Sources/NIOFileSystem/FileHandleProtocol.swift +++ b/Sources/NIOFileSystem/FileHandleProtocol.swift @@ -354,7 +354,56 @@ extension ReadableFileHandleProtocol { } let isSeekable = !(info.type == .fifo || info.type == .socket) - guard isSeekable else { + if isSeekable { + // Limit the size of single shot reads. If the system is busy then avoid slowing down other + // work by blocking an I/O executor thread while doing a large read. The limit is somewhat + // arbitrary. + let singleShotReadLimit = 64 * 1024 * 1024 + + // If the file size isn't 0 but the read size is, then it means that + // we are intending to read an empty fragment: we can just return + // fast and skip any reads. + // If the file size is 0, we can't conclude anything about the size + // of the file, as `stat` will return 0 for unbounded files. + // If this happens, just read in chunks to make sure the whole file + // is read (up to `maximumSizeAllowed` bytes). + var forceChunkedRead = false + if fileSize > 0 { + if readSize == 0 { + return ByteBuffer() + } + } else { + forceChunkedRead = true + } + + if !forceChunkedRead, readSize <= singleShotReadLimit { + return try await self.readChunk( + fromAbsoluteOffset: offset, + length: .bytes(Int64(readSize)) + ) + } else { + var accumulator = ByteBuffer() + accumulator.reserveCapacity(readSize) + + for try await chunk in self.readChunks(in: offset..., chunkLength: .mebibytes(8)) { + accumulator.writeImmutableBuffer(chunk) + if accumulator.readableBytes > maximumSizeAllowed.bytes { + throw FileSystemError( + code: .resourceExhausted, + message: """ + There are more bytes to read than the maximum size allowed \ + (\(maximumSizeAllowed)). Read the file in chunks or increase the maximum size \ + allowed. + """, + cause: nil, + location: .here() + ) + } + } + + return accumulator + } + } else { guard offset == 0 else { throw FileSystemError( code: .unsupported, @@ -384,53 +433,6 @@ extension ReadableFileHandleProtocol { return accumulator } - // Limit the size of single shot reads. If the system is busy then avoid slowing down other - // work by blocking an I/O executor thread while doing a large read. The limit is somewhat - // arbitrary. - let singleShotReadLimit = 64 * 1024 * 1024 - - // If the file size isn't 0 but the read size is, then it means that - // we are intending to read an empty fragment: we can just return - // fast and skip any reads. - // If the file size is 0, we can't conclude anything about the size - // of the file, as `stat` will return 0 for unbounded files. - // If this happens, just read in chunks to make sure the whole file - // is read (up to `maximumSizeAllowed` bytes). - var forceChunkedRead = false - if fileSize > 0 { - if readSize == 0 { - return ByteBuffer() - } - } else { - forceChunkedRead = true - } - - guard !forceChunkedRead, readSize <= singleShotReadLimit else { - var accumulator = ByteBuffer() - accumulator.reserveCapacity(readSize) - - for try await chunk in self.readChunks(in: offset..., chunkLength: .mebibytes(8)) { - accumulator.writeImmutableBuffer(chunk) - if accumulator.readableBytes > maximumSizeAllowed.bytes { - throw FileSystemError( - code: .resourceExhausted, - message: """ - There are more bytes to read than the maximum size allowed \ - (\(maximumSizeAllowed)). Read the file in chunks or increase the maximum size \ - allowed. - """, - cause: nil, - location: .here() - ) - } - } - - return accumulator - } - return try await self.readChunk( - fromAbsoluteOffset: offset, - length: .bytes(Int64(readSize)) - ) } } diff --git a/Sources/NIOFileSystem/FileSystem.swift b/Sources/NIOFileSystem/FileSystem.swift index bba8af9a3e..16692a7854 100644 --- a/Sources/NIOFileSystem/FileSystem.swift +++ b/Sources/NIOFileSystem/FileSystem.swift @@ -820,12 +820,13 @@ extension FileSystem { } // Drop the last component and loop around. - guard let component = path.lastComponent else { + if let component = path.lastComponent { + path.removeLastComponent() + droppedComponents.append(component) + } else { // Should only happen if the path is empty or contains just the root. return .failure(.mkdir(errno: errno, path: path, location: .here())) } - path.removeLastComponent() - droppedComponents.append(component) } } @@ -858,11 +859,12 @@ extension FileSystem { return result.map { FileInfo(platformSpecificStatus: $0) }.flatMapError { errno in - guard errno == .noSuchFileOrDirectory else { + if errno == .noSuchFileOrDirectory { + return .success(nil) + } else { let name = infoAboutSymbolicLink ? "lstat" : "stat" return .failure(.stat(name, errno: errno, path: path, location: .here())) } - return .success(nil) } } @@ -1315,18 +1317,19 @@ extension FileSystem { case .success: return .success(finalPath) case let .failure(error): - guard let systemCallError = error.cause as? FileSystemError.SystemCallError else { - let fileSystemError = FileSystemError( - message: "Unable to create temporary directory '\(template)'.", - wrapping: error - ) - return .failure(fileSystemError) - } - switch systemCallError.errno { - // If the file at the generated path already exists, we generate a new file path. - case .fileExists, .isDirectory: - break - default: + if let systemCallError = error.cause as? FileSystemError.SystemCallError { + switch systemCallError.errno { + // If the file at the generated path already exists, we generate a new file path. + case .fileExists, .isDirectory: + break + default: + let fileSystemError = FileSystemError( + message: "Unable to create temporary directory '\(template)'.", + wrapping: error + ) + return .failure(fileSystemError) + } + } else { let fileSystemError = FileSystemError( message: "Unable to create temporary directory '\(template)'.", wrapping: error diff --git a/Sources/NIOFileSystem/FileSystemError.swift b/Sources/NIOFileSystem/FileSystemError.swift index 0408422707..801fcd699c 100644 --- a/Sources/NIOFileSystem/FileSystemError.swift +++ b/Sources/NIOFileSystem/FileSystemError.swift @@ -60,22 +60,24 @@ public struct FileSystemError: Error, Sendable { extension FileSystemError: CustomStringConvertible { public var description: String { - guard let cause = self.cause else { + if let cause = self.cause { + return "\(self.code): \(self.message) (\(cause))" + } else { return "\(self.code): \(self.message)" } - return "\(self.code): \(self.message) (\(cause))" } } extension FileSystemError: CustomDebugStringConvertible { public var debugDescription: String { - guard let cause = self.cause else { + if let cause = self.cause { + return """ + \(String(reflecting: self.code)): \(String(reflecting: self.message)) \ + (\(String(reflecting: cause))) + """ + } else { return "\(String(reflecting: self.code)): \(String(reflecting: self.message))" } - return """ - \(String(reflecting: self.code)): \(String(reflecting: self.message)) \ - (\(String(reflecting: cause))) - """ } } diff --git a/Sources/NIOFileSystem/Internal/BufferedStream.swift b/Sources/NIOFileSystem/Internal/BufferedStream.swift index 6c8a70e2d5..6653e4f8c0 100644 --- a/Sources/NIOFileSystem/Internal/BufferedStream.swift +++ b/Sources/NIOFileSystem/Internal/BufferedStream.swift @@ -998,18 +998,23 @@ extension BufferedStream { mutating func sequenceDeinitialized() -> SequenceDeinitializedAction? { switch self._state { case .initial(let initial): - guard initial.iteratorInitialized else { + if initial.iteratorInitialized { + // An iterator was created and we deinited the sequence. + // This is an expected pattern and we just continue on normal. + return .none + } else { // No iterator was created so we can transition to finished right away. self._state = .finished(iteratorInitialized: false) return .callOnTermination(initial.onTermination) } - // An iterator was created and we deinited the sequence. - // This is an expected pattern and we just continue on normal. - return .none case .streaming(let streaming): - guard streaming.iteratorInitialized else { + if streaming.iteratorInitialized { + // An iterator was created and we deinited the sequence. + // This is an expected pattern and we just continue on normal. + return .none + } else { // No iterator was created so we can transition to finished right away. self._state = .finished(iteratorInitialized: false) @@ -1018,20 +1023,18 @@ extension BufferedStream { streaming.onTermination ) } - // An iterator was created and we deinited the sequence. - // This is an expected pattern and we just continue on normal. - return .none case .sourceFinished(let sourceFinished): - guard sourceFinished.iteratorInitialized else { + if sourceFinished.iteratorInitialized { + // An iterator was created and we deinited the sequence. + // This is an expected pattern and we just continue on normal. + return .none + } else { // No iterator was created so we can transition to finished right away. self._state = .finished(iteratorInitialized: false) return .callOnTermination(sourceFinished.onTermination) } - // An iterator was created and we deinited the sequence. - // This is an expected pattern and we just continue on normal. - return .none case .finished: // We are already finished so there is nothing left to clean up. @@ -1171,7 +1174,15 @@ extension BufferedStream { return .callOnTermination(initial.onTermination) case .streaming(let streaming): - guard streaming.buffer.isEmpty else { + if streaming.buffer.isEmpty { + // We can transition to finished right away since the buffer is empty now + self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) + + return .failProducersAndCallOnTermination( + Array(streaming.producerContinuations.map { $0.1 }), + streaming.onTermination + ) + } else { // The continuation must be `nil` if the buffer has elements precondition(streaming.consumerContinuation == nil) @@ -1188,13 +1199,6 @@ extension BufferedStream { Array(streaming.producerContinuations.map { $0.1 }) ) } - // We can transition to finished right away since the buffer is empty now - self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) - - return .failProducersAndCallOnTermination( - Array(streaming.producerContinuations.map { $0.1 }), - streaming.onTermination - ) case .sourceFinished, .finished: // This is normal and we just have to tolerate it @@ -1292,27 +1296,28 @@ extension BufferedStream { streaming.hasOutstandingDemand = shouldProduceMore let callbackToken = shouldProduceMore ? nil : self.nextCallbackToken() - guard let consumerContinuation = streaming.consumerContinuation else { + if let consumerContinuation = streaming.consumerContinuation { + guard let element = streaming.buffer.popFirst() else { + // We got a yield of an empty sequence. We just tolerate this. + self._state = .streaming(streaming) + + return .init(callbackToken: callbackToken) + } + + // We got a consumer continuation and an element. We can resume the consumer now + streaming.consumerContinuation = nil + self._state = .streaming(streaming) + return .init( + callbackToken: callbackToken, + continuationAndElement: (consumerContinuation, element) + ) + } else { // We don't have a suspended consumer so we just buffer the elements self._state = .streaming(streaming) return .init( callbackToken: callbackToken ) } - guard let element = streaming.buffer.popFirst() else { - // We got a yield of an empty sequence. We just tolerate this. - self._state = .streaming(streaming) - - return .init(callbackToken: callbackToken) - } - - // We got a consumer continuation and an element. We can resume the consumer now - streaming.consumerContinuation = nil - self._state = .streaming(streaming) - return .init( - callbackToken: callbackToken, - continuationAndElement: (consumerContinuation, element) - ) case .sourceFinished, .finished: // If the source has finished we are dropping the elements. @@ -1386,11 +1391,16 @@ extension BufferedStream { fatalError("AsyncStream internal inconsistency") case .streaming(var streaming): - guard - let index = streaming.producerContinuations.firstIndex(where: { - $0.0 == callbackToken.id - }) - else { + if let index = streaming.producerContinuations.firstIndex(where: { + $0.0 == callbackToken.id + }) { + // We have an enqueued producer that we need to resume now + self._state = .modify + let continuation = streaming.producerContinuations.remove(at: index).1 + self._state = .streaming(streaming) + + return .resumeProducerWithCancellationError(continuation) + } else { // The task that yields was cancelled before yielding so the cancellation handler // got invoked right away self._state = .modify @@ -1399,12 +1409,6 @@ extension BufferedStream { return .none } - // We have an enqueued producer that we need to resume now - self._state = .modify - let continuation = streaming.producerContinuations.remove(at: index).1 - self._state = .streaming(streaming) - - return .resumeProducerWithCancellationError(continuation) case .sourceFinished, .finished: // Since we are unlocking between yielding and suspending the yield @@ -1451,7 +1455,24 @@ extension BufferedStream { return .callOnTermination(initial.onTermination) case .streaming(let streaming): - guard let consumerContinuation = streaming.consumerContinuation else { + if let consumerContinuation = streaming.consumerContinuation { + // We have a continuation, this means our buffer must be empty + // Furthermore, we can now transition to finished + // and resume the continuation with the failure + precondition(streaming.buffer.isEmpty, "Expected an empty buffer") + precondition( + streaming.producerContinuations.isEmpty, + "Expected no suspended producers" + ) + + self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) + + return .resumeConsumerAndCallOnTermination( + consumerContinuation: consumerContinuation, + failure: failure, + onTermination: streaming.onTermination + ) + } else { self._state = .sourceFinished( .init( iteratorInitialized: streaming.iteratorInitialized, @@ -1465,22 +1486,6 @@ extension BufferedStream { producerContinuations: Array(streaming.producerContinuations.map { $0.1 }) ) } - // We have a continuation, this means our buffer must be empty - // Furthermore, we can now transition to finished - // and resume the continuation with the failure - precondition(streaming.buffer.isEmpty, "Expected an empty buffer") - precondition( - streaming.producerContinuations.isEmpty, - "Expected no suspended producers" - ) - - self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) - - return .resumeConsumerAndCallOnTermination( - consumerContinuation: consumerContinuation, - failure: failure, - onTermination: streaming.onTermination - ) case .sourceFinished, .finished: // If the source has finished, finishing again has no effect. @@ -1532,7 +1537,25 @@ extension BufferedStream { self._state = .modify - guard let element = streaming.buffer.popFirst() else { + if let element = streaming.buffer.popFirst() { + // We have an element to fulfil the demand right away. + let shouldProduceMore = streaming.backPressureStrategy.didConsume( + bufferDepth: streaming.buffer.count + ) + streaming.hasOutstandingDemand = shouldProduceMore + + if shouldProduceMore { + // There is demand and we have to resume our producers + let producers = Array(streaming.producerContinuations.map { $0.1 }) + streaming.producerContinuations.removeAll() + self._state = .streaming(streaming) + return .returnElementAndResumeProducers(element, producers) + } else { + // We don't have any new demand, so we can just return the element. + self._state = .streaming(streaming) + return .returnElement(element) + } + } else { // There is nothing in the buffer to fulfil the demand so we need to suspend. // We are not interacting with the back-pressure strategy here because // we are doing this inside `suspendNext` @@ -1540,28 +1563,16 @@ extension BufferedStream { return .suspendTask } - // We have an element to fulfil the demand right away. - let shouldProduceMore = streaming.backPressureStrategy.didConsume( - bufferDepth: streaming.buffer.count - ) - streaming.hasOutstandingDemand = shouldProduceMore - - guard shouldProduceMore else { - // We don't have any new demand, so we can just return the element. - self._state = .streaming(streaming) - return .returnElement(element) - } - // There is demand and we have to resume our producers - let producers = Array(streaming.producerContinuations.map { $0.1 }) - streaming.producerContinuations.removeAll() - self._state = .streaming(streaming) - return .returnElementAndResumeProducers(element, producers) case .sourceFinished(var sourceFinished): // Check if we have an element left in the buffer and return it self._state = .modify - guard let element = sourceFinished.buffer.popFirst() else { + if let element = sourceFinished.buffer.popFirst() { + self._state = .sourceFinished(sourceFinished) + + return .returnElement(element) + } else { // We are returning the queued failure now and can transition to finished self._state = .finished(iteratorInitialized: sourceFinished.iteratorInitialized) @@ -1570,9 +1581,6 @@ extension BufferedStream { sourceFinished.onTermination ) } - self._state = .sourceFinished(sourceFinished) - - return .returnElement(element) case .finished: return .returnNil @@ -1621,40 +1629,46 @@ extension BufferedStream { self._state = .modify // We have to check here again since we might have a producer interleave next and suspendNext - guard let element = streaming.buffer.popFirst() else { + if let element = streaming.buffer.popFirst() { + // We have an element to fulfil the demand right away. + + let shouldProduceMore = streaming.backPressureStrategy.didConsume( + bufferDepth: streaming.buffer.count + ) + streaming.hasOutstandingDemand = shouldProduceMore + + if shouldProduceMore { + // There is demand and we have to resume our producers + let producers = Array(streaming.producerContinuations.map { $0.1 }) + streaming.producerContinuations.removeAll() + self._state = .streaming(streaming) + return .resumeConsumerWithElementAndProducers( + continuation, + element, + producers + ) + } else { + // We don't have any new demand, so we can just return the element. + self._state = .streaming(streaming) + return .resumeConsumerWithElement(continuation, element) + } + } else { // There is nothing in the buffer to fulfil the demand so we to store the continuation. streaming.consumerContinuation = continuation self._state = .streaming(streaming) return .none } - // We have an element to fulfil the demand right away. - - let shouldProduceMore = streaming.backPressureStrategy.didConsume( - bufferDepth: streaming.buffer.count - ) - streaming.hasOutstandingDemand = shouldProduceMore - - guard shouldProduceMore else { - // We don't have any new demand, so we can just return the element. - self._state = .streaming(streaming) - return .resumeConsumerWithElement(continuation, element) - } - // There is demand and we have to resume our producers - let producers = Array(streaming.producerContinuations.map { $0.1 }) - streaming.producerContinuations.removeAll() - self._state = .streaming(streaming) - return .resumeConsumerWithElementAndProducers( - continuation, - element, - producers - ) case .sourceFinished(var sourceFinished): // Check if we have an element left in the buffer and return it self._state = .modify - guard let element = sourceFinished.buffer.popFirst() else { + if let element = sourceFinished.buffer.popFirst() { + self._state = .sourceFinished(sourceFinished) + + return .resumeConsumerWithElement(continuation, element) + } else { // We are returning the queued failure now and can transition to finished self._state = .finished(iteratorInitialized: sourceFinished.iteratorInitialized) @@ -1664,9 +1678,6 @@ extension BufferedStream { sourceFinished.onTermination ) } - self._state = .sourceFinished(sourceFinished) - - return .resumeConsumerWithElement(continuation, element) case .finished: return .resumeConsumerWithNil(continuation) @@ -1696,20 +1707,21 @@ extension BufferedStream { case .streaming(let streaming): self._state = .finished(iteratorInitialized: streaming.iteratorInitialized) - guard let consumerContinuation = streaming.consumerContinuation else { + if let consumerContinuation = streaming.consumerContinuation { + precondition( + streaming.producerContinuations.isEmpty, + "Internal inconsistency. Unexpected producer continuations." + ) + return .resumeConsumerWithCancellationErrorAndCallOnTermination( + consumerContinuation, + streaming.onTermination + ) + } else { return .failProducersAndCallOnTermination( Array(streaming.producerContinuations.map { $0.1 }), streaming.onTermination ) } - precondition( - streaming.producerContinuations.isEmpty, - "Internal inconsistency. Unexpected producer continuations." - ) - return .resumeConsumerWithCancellationErrorAndCallOnTermination( - consumerContinuation, - streaming.onTermination - ) case .sourceFinished, .finished: return .none diff --git a/Sources/NIOFileSystem/Internal/System Calls/Errno.swift b/Sources/NIOFileSystem/Internal/System Calls/Errno.swift index a3c47d7289..0ac8876aff 100644 --- a/Sources/NIOFileSystem/Internal/System Calls/Errno.swift +++ b/Sources/NIOFileSystem/Internal/System Calls/Errno.swift @@ -70,14 +70,16 @@ public func valueOrErrno( while true { Errno.clear() let result = fn() - guard result == -1 else { + if result == -1 { + let errno = Errno._current + if errno == .interrupted, retryOnInterrupt { + continue + } else { + return .failure(errno) + } + } else { return .success(result) } - let errno = Errno._current - guard errno == .interrupted, retryOnInterrupt else { - return .failure(errno) - } - continue } } diff --git a/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift b/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift index 4ba64d0450..d49c549056 100644 --- a/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift +++ b/Sources/NIOFileSystem/Internal/System Calls/FileDescriptor+Syscalls.swift @@ -51,11 +51,12 @@ extension FileDescriptor { let oFlag = mode.rawValue | options.rawValue let rawValue = valueOrErrno(retryOnInterrupt: retryOnInterrupt) { path.withPlatformString { - guard let permissions = permissions else { + if let permissions = permissions { + return system_fdopenat(self.rawValue, $0, oFlag, permissions.rawValue) + } else { precondition(!options.contains(.create), "Create must be given permissions") return system_fdopenat(self.rawValue, $0, oFlag) } - return system_fdopenat(self.rawValue, $0, oFlag, permissions.rawValue) } } @@ -278,10 +279,11 @@ extension FileDescriptor { bufferPointer = buffer } - guard let offset else { + if let offset { + return try self.read(fromAbsoluteOffset: offset, into: bufferPointer) + } else { return try self.read(into: bufferPointer) } - return try self.read(fromAbsoluteOffset: offset, into: bufferPointer) } return buffer } diff --git a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift index 7037fbaf9a..6f560c0de0 100644 --- a/Sources/NIOFileSystem/Internal/SystemFileHandle.swift +++ b/Sources/NIOFileSystem/Internal/SystemFileHandle.swift @@ -161,10 +161,11 @@ extension SystemFileHandle.SendableView { _ execute: (FileDescriptor) throws -> R, onUnavailable: () -> FileSystemError ) throws -> R { - guard let descriptor = self.descriptorIfAvailable() else { + if let descriptor = self.descriptorIfAvailable() { + return try execute(descriptor) + } else { throw onUnavailable() } - return try execute(descriptor) } /// Executes a closure with the file descriptor if it's available otherwise returns the result @@ -173,10 +174,11 @@ extension SystemFileHandle.SendableView { _ execute: (FileDescriptor) -> Result, onUnavailable: () -> FileSystemError ) -> Result { - guard let descriptor = self.descriptorIfAvailable() else { + if let descriptor = self.descriptorIfAvailable() { + return execute(descriptor) + } else { return .failure(onUnavailable()) } - return execute(descriptor) } } @@ -736,10 +738,11 @@ extension SystemFileHandle.SendableView { let linkAtResult = linkAtEmptyPath().flatMapError { errno in // ENOENT means we likely didn't have the 'CAP_DAC_READ_SEARCH' capability // so try again by linking to the descriptor via procfs. - guard errno == .noSuchFileOrDirectory else { + if errno == .noSuchFileOrDirectory { + return linkAtProcFS() + } else { return .failure(errno) } - return linkAtProcFS() }.mapError { errno in FileSystemError.link( errno: errno, @@ -753,7 +756,24 @@ extension SystemFileHandle.SendableView { case .failure(.noSuchFileOrDirectory): result = linkAtProcFS().flatMapError { errno in - guard errno == .fileExists, !exclusive else { + if errno == .fileExists, !exclusive { + return Libc.remove(desiredPath).mapError { errno in + FileSystemError.remove( + errno: errno, + path: desiredPath, + location: .here() + ) + }.flatMap { + linkAtProcFS().mapError { errno in + FileSystemError.link( + errno: errno, + from: createdPath, + to: desiredPath, + location: .here() + ) + } + } + } else { let error = FileSystemError.link( errno: errno, from: createdPath, @@ -762,22 +782,6 @@ extension SystemFileHandle.SendableView { ) return .failure(error) } - return Libc.remove(desiredPath).mapError { errno in - FileSystemError.remove( - errno: errno, - path: desiredPath, - location: .here() - ) - }.flatMap { - linkAtProcFS().mapError { errno in - FileSystemError.link( - errno: errno, - from: createdPath, - to: desiredPath, - location: .here() - ) - } - } } case .failure(let errno): @@ -1025,35 +1029,36 @@ extension SystemFileHandle: ReadableFileHandleProtocol { fromAbsoluteOffset: offset, length: length.bytes ).flatMapError { error in - guard let errno = error as? Errno, errno == .illegalSeek else { - return .failure( + if let errno = error as? Errno, errno == .illegalSeek { + guard offset == 0 else { + return .failure( + FileSystemError( + code: .unsupported, + message: "File is unseekable.", + cause: nil, + location: .here() + ) + ) + } + + return descriptor.readChunk(length: length.bytes).mapError { error in FileSystemError.read( - usingSyscall: .pread, + usingSyscall: .read, error: error, path: sendableView.path, location: .here() ) - ) - } - guard offset == 0 else { + } + } else { return .failure( - FileSystemError( - code: .unsupported, - message: "File is unseekable.", - cause: nil, + FileSystemError.read( + usingSyscall: .pread, + error: error, + path: sendableView.path, location: .here() ) ) } - - return descriptor.readChunk(length: length.bytes).mapError { error in - FileSystemError.read( - usingSyscall: .read, - error: error, - path: sendableView.path, - location: .here() - ) - } } .get() } onUnavailable: { @@ -1088,7 +1093,28 @@ extension SystemFileHandle: WritableFileHandleProtocol { try sendableView._withUnsafeDescriptor { descriptor in try descriptor.write(contentsOf: bytes, toAbsoluteOffset: offset) .flatMapError { error in - guard let errno = error as? Errno, errno == .illegalSeek else { + if let errno = error as? Errno, errno == .illegalSeek { + guard offset == 0 else { + return .failure( + FileSystemError( + code: .unsupported, + message: "File is unseekable.", + cause: nil, + location: .here() + ) + ) + } + + return descriptor.write(contentsOf: bytes) + .mapError { error in + FileSystemError.write( + usingSyscall: .write, + error: error, + path: sendableView.path, + location: .here() + ) + } + } else { return .failure( FileSystemError.write( usingSyscall: .pwrite, @@ -1098,26 +1124,6 @@ extension SystemFileHandle: WritableFileHandleProtocol { ) ) } - guard offset == 0 else { - return .failure( - FileSystemError( - code: .unsupported, - message: "File is unseekable.", - cause: nil, - location: .here() - ) - ) - } - - return descriptor.write(contentsOf: bytes) - .mapError { error in - FileSystemError.write( - usingSyscall: .write, - error: error, - path: sendableView.path, - location: .here() - ) - } } .get() } onUnavailable: { @@ -1330,7 +1336,27 @@ extension SystemFileHandle { let truncate = options.contains(.truncate) let delayMaterialization = transactional && isWritable && (exclusiveCreate || truncate) - guard delayMaterialization else { + if delayMaterialization { + // When opening in this mode we can more "atomically" create the file, that is, by not + // leaving the user with a half written file should e.g. the system crash or throw an + // error while writing. On non-Android Linux we do this by opening the directory for + // the path with `O_TMPFILE` and creating a hard link when closing the file. On other + // platforms we generate a dot file with a randomised suffix name and rename it to the + // destination. + #if os(Android) + let temporaryHardLink = false + #else + let temporaryHardLink = true + #endif + return Self.syncOpenWithMaterialization( + atPath: path, + mode: mode, + options: options, + permissions: permissions, + threadPool: threadPool, + useTemporaryFileIfPossible: temporaryHardLink + ) + } else { return Self.syncOpen( atPath: path, mode: mode, @@ -1339,25 +1365,6 @@ extension SystemFileHandle { threadPool: threadPool ) } - // When opening in this mode we can more "atomically" create the file, that is, by not - // leaving the user with a half written file should e.g. the system crash or throw an - // error while writing. On non-Android Linux we do this by opening the directory for - // the path with `O_TMPFILE` and creating a hard link when closing the file. On other - // platforms we generate a dot file with a randomised suffix name and rename it to the - // destination. - #if os(Android) - let temporaryHardLink = false - #else - let temporaryHardLink = true - #endif - return Self.syncOpenWithMaterialization( - atPath: path, - mode: mode, - options: options, - permissions: permissions, - threadPool: threadPool, - useTemporaryFileIfPossible: temporaryHardLink - ) } static func syncOpen( diff --git a/Sources/NIOFileSystem/OpenOptions.swift b/Sources/NIOFileSystem/OpenOptions.swift index 1bf901c553..454826897a 100644 --- a/Sources/NIOFileSystem/OpenOptions.swift +++ b/Sources/NIOFileSystem/OpenOptions.swift @@ -199,10 +199,11 @@ extension OpenOptions { extension OpenOptions.Write { @_spi(Testing) public var permissionsForRegularFile: FilePermissions? { - guard let newFile = self.newFile else { + if let newFile = self.newFile { + return newFile.permissions ?? .defaultsForRegularFile + } else { return nil } - return newFile.permissions ?? .defaultsForRegularFile } var descriptorOptions: FileDescriptor.OpenOptions { diff --git a/Sources/NIOFoundationCompat/ByteBuffer-foundation.swift b/Sources/NIOFoundationCompat/ByteBuffer-foundation.swift index 452862ee45..7aedfb17b0 100644 --- a/Sources/NIOFoundationCompat/ByteBuffer-foundation.swift +++ b/Sources/NIOFoundationCompat/ByteBuffer-foundation.swift @@ -120,7 +120,12 @@ extension ByteBuffer { } return self.withUnsafeReadableBytesWithStorageManagement { ptr, storageRef in - guard doCopy else { + if doCopy { + return Data( + bytes: UnsafeMutableRawPointer(mutating: ptr.baseAddress!.advanced(by: index)), + count: Int(length) + ) + } else { _ = storageRef.retain() return Data( bytesNoCopy: UnsafeMutableRawPointer(mutating: ptr.baseAddress!.advanced(by: index)), @@ -128,10 +133,6 @@ extension ByteBuffer { deallocator: .custom { _, _ in storageRef.release() } ) } - return Data( - bytes: UnsafeMutableRawPointer(mutating: ptr.baseAddress!.advanced(by: index)), - count: Int(length) - ) } } diff --git a/Sources/NIOHTTP1/ByteCollectionUtils.swift b/Sources/NIOHTTP1/ByteCollectionUtils.swift index aeb30328b0..95b6f8dff5 100644 --- a/Sources/NIOHTTP1/ByteCollectionUtils.swift +++ b/Sources/NIOHTTP1/ByteCollectionUtils.swift @@ -59,10 +59,11 @@ extension Sequence where Self.Element == UInt8 { } } - guard let maybeResult = maybeMaybeResult, let result = maybeResult else { + if let maybeResult = maybeMaybeResult, let result = maybeResult { + return result + } else { return self.elementsEqual(to, by: { ($0 & 0xdf) == ($1 & 0xdf) }) } - return result } } diff --git a/Sources/NIOHTTP1/HTTPDecoder.swift b/Sources/NIOHTTP1/HTTPDecoder.swift index 036113e25f..eb33c620ba 100644 --- a/Sources/NIOHTTP1/HTTPDecoder.swift +++ b/Sources/NIOHTTP1/HTTPDecoder.swift @@ -430,13 +430,18 @@ private class BetterHTTPParser { // If we have a richer error than the errno code, and the errno is internal, we'll use it. Otherwise, we use the // error from http_parser. let err = self.httpErrno ?? parserErrno - guard err == HPE_INTERNAL || err == HPE_USER, let richerError = self.richerError else { + if err == HPE_INTERNAL || err == HPE_USER, let richerError = self.richerError { + throw richerError + } else { throw HTTPParserError.httpError(fromCHTTPParserErrno: err)! } - throw richerError } - guard let firstNonDiscardableOffset = self.firstNonDiscardableOffset else { + if let firstNonDiscardableOffset = self.firstNonDiscardableOffset { + self.httpParserOffset += bytesRead - firstNonDiscardableOffset + self.firstNonDiscardableOffset = 0 + return firstNonDiscardableOffset + } else { // By definition we've consumed all of the http parser offset at this stage. There may still be bytes // left in the buffer though: we didn't consume them because they aren't ours to consume, as they may belong // to an upgraded protocol. @@ -447,9 +452,6 @@ private class BetterHTTPParser { self.httpParserOffset = 0 return consumedBytes } - self.httpParserOffset += bytesRead - firstNonDiscardableOffset - self.firstNonDiscardableOffset = 0 - return firstNonDiscardableOffset } } diff --git a/Sources/NIOHTTP1/HTTPEncoder.swift b/Sources/NIOHTTP1/HTTPEncoder.swift index 345ba87cdd..19bc6109ac 100644 --- a/Sources/NIOHTTP1/HTTPEncoder.swift +++ b/Sources/NIOHTTP1/HTTPEncoder.swift @@ -135,11 +135,12 @@ private func correctlyFrameTransportHeaders( if headers.contains(name: "content-length") { return .contentLength } - guard version.major == 1 && version.minor >= 1 else { + if version.major == 1 && version.minor >= 1 { + headers.replaceOrAdd(name: "transfer-encoding", value: "chunked") + return .chunked + } else { return .neither } - headers.replaceOrAdd(name: "transfer-encoding", value: "chunked") - return .chunked case .unlikely: if headers.contains(name: "content-length") { return .contentLength @@ -154,10 +155,11 @@ private func correctlyFrameTransportHeaders( /// Validates the response/request headers to ensure that we correctly send the body with chunked transfer /// encoding, when needed. private func messageIsChunked(headers: HTTPHeaders, version: HTTPVersion) -> Bool { - guard version.major == 1 && version.minor >= 1 else { + if version.major == 1 && version.minor >= 1 { + return headers.first(name: "transfer-encoding") == "chunked" ? true : false + } else { return false } - return headers.first(name: "transfer-encoding") == "chunked" ? true : false } /// A `ChannelOutboundHandler` that can serialize HTTP requests. diff --git a/Sources/NIOHTTP1/HTTPHeaders+Validation.swift b/Sources/NIOHTTP1/HTTPHeaders+Validation.swift index 57a3c2ba3f..b5a29c695f 100644 --- a/Sources/NIOHTTP1/HTTPHeaders+Validation.swift +++ b/Sources/NIOHTTP1/HTTPHeaders+Validation.swift @@ -43,10 +43,11 @@ extension String { let fastResult = self.utf8.withContiguousStorageIfAvailable { ptr in ptr.allSatisfy { $0.isValidHeaderFieldValueByte } } - guard let fastResult = fastResult else { + if let fastResult = fastResult { + return fastResult + } else { return self.utf8._isValidHeaderFieldValue_slowPath } - return fastResult } /// Validates a given header field name against the definition in RFC 9110. @@ -70,10 +71,11 @@ extension String { let fastResult = self.utf8.withContiguousStorageIfAvailable { ptr in ptr.allSatisfy { $0.isValidHeaderFieldNameByte } } - guard let fastResult = fastResult else { + if let fastResult = fastResult { + return fastResult + } else { return self.utf8._isValidHeaderFieldName_slowPath } - return fastResult } } diff --git a/Sources/NIOHTTP1/NIOHTTPObjectAggregator.swift b/Sources/NIOHTTP1/NIOHTTPObjectAggregator.swift index b0b53e8e47..3821e6e2b4 100644 --- a/Sources/NIOHTTP1/NIOHTTPObjectAggregator.swift +++ b/Sources/NIOHTTP1/NIOHTTPObjectAggregator.swift @@ -256,11 +256,12 @@ public final class NIOHTTPServerRequestAggregator: ChannelInboundHandler, Remova content: inout ByteBuffer, message: InboundIn ) -> HTTPResponseHead? { - guard content.readableBytes > self.maxContentLength - self.buffer.readableBytes else { + if content.readableBytes > self.maxContentLength - self.buffer.readableBytes { + return self.handleOversizeMessage(message: message) + } else { self.buffer.writeBuffer(&content) return nil } - return self.handleOversizeMessage(message: message) } private func endAggregation(context: ChannelHandlerContext, trailingHeaders: HTTPHeaders?) { diff --git a/Sources/NIOHTTP1/NIOTypedHTTPClientUpgraderStateMachine.swift b/Sources/NIOHTTP1/NIOTypedHTTPClientUpgraderStateMachine.swift index 86c0d69f21..be51bd22f5 100644 --- a/Sources/NIOHTTP1/NIOTypedHTTPClientUpgraderStateMachine.swift +++ b/Sources/NIOHTTP1/NIOTypedHTTPClientUpgraderStateMachine.swift @@ -273,23 +273,25 @@ struct NIOTypedHTTPClientUpgraderStateMachine { case .upgrading(let upgrading): switch result { case .success(let value): - guard !upgrading.buffer.isEmpty else { + if !upgrading.buffer.isEmpty { + self.state = .unbuffering(.init(buffer: upgrading.buffer)) + return .startUnbuffering(value) + } else { self.state = .finished return .removeHandler(value) } - self.state = .unbuffering(.init(buffer: upgrading.buffer)) - return .startUnbuffering(value) case .failure(let error): - guard !upgrading.buffer.isEmpty else { + if !upgrading.buffer.isEmpty { + // So we failed to upgrade. There is nothing really that we can do here. + // We are unbuffering the reads but there shouldn't be any handler in the pipeline + // that expects a specific type of reads anyhow. + self.state = .unbuffering(.init(buffer: upgrading.buffer)) + return .fireErrorCaughtAndStartUnbuffering(error) + } else { self.state = .finished return .fireErrorCaughtAndRemoveHandler(error) } - // So we failed to upgrade. There is nothing really that we can do here. - // We are unbuffering the reads but there shouldn't be any handler in the pipeline - // that expects a specific type of reads anyhow. - self.state = .unbuffering(.init(buffer: upgrading.buffer)) - return .fireErrorCaughtAndStartUnbuffering(error) } case .finished: @@ -316,14 +318,15 @@ struct NIOTypedHTTPClientUpgraderStateMachine { case .unbuffering(var unbuffering): self.state = .modifying - guard let element = unbuffering.buffer.popFirst() else { + if let element = unbuffering.buffer.popFirst() { + self.state = .unbuffering(unbuffering) + + return .fireChannelRead(element) + } else { self.state = .finished return .fireChannelReadCompleteAndRemoveHandler } - self.state = .unbuffering(unbuffering) - - return .fireChannelRead(element) case .modifying: fatalError("Internal inconsistency in HTTPClientUpgradeStateMachine") diff --git a/Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift b/Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift index 4aac100757..80fd018944 100644 --- a/Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift +++ b/Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift @@ -96,15 +96,16 @@ struct NIOTypedHTTPServerUpgraderStateMachine { return .unwrapData case .awaitingUpgrader(var awaitingUpgrader): - guard awaitingUpgrader.seenFirstRequest else { + if awaitingUpgrader.seenFirstRequest { + // We should buffer the data since we have seen the full request. + self.state = .modifying + awaitingUpgrader.buffer.append(data) + self.state = .awaitingUpgrader(awaitingUpgrader) + return nil + } else { // We shouldn't buffer. This means we are still expecting HTTP parts. return .unwrapData } - // We should buffer the data since we have seen the full request. - self.state = .modifying - awaitingUpgrader.buffer.append(data) - self.state = .awaitingUpgrader(awaitingUpgrader) - return nil case .upgraderReady: // We have not seen the end of the HTTP request so this @@ -257,23 +258,25 @@ struct NIOTypedHTTPServerUpgraderStateMachine { case .upgrading(let upgrading): switch result { case .success(let value): - guard !upgrading.buffer.isEmpty else { + if !upgrading.buffer.isEmpty { + self.state = .unbuffering(.init(buffer: upgrading.buffer)) + return .startUnbuffering(value) + } else { self.state = .finished return .removeHandler(value) } - self.state = .unbuffering(.init(buffer: upgrading.buffer)) - return .startUnbuffering(value) case .failure(let error): - guard !upgrading.buffer.isEmpty else { + if !upgrading.buffer.isEmpty { + // So we failed to upgrade. There is nothing really that we can do here. + // We are unbuffering the reads but there shouldn't be any handler in the pipeline + // that expects a specific type of reads anyhow. + self.state = .unbuffering(.init(buffer: upgrading.buffer)) + return .fireErrorCaughtAndStartUnbuffering(error) + } else { self.state = .finished return .fireErrorCaughtAndRemoveHandler(error) } - // So we failed to upgrade. There is nothing really that we can do here. - // We are unbuffering the reads but there shouldn't be any handler in the pipeline - // that expects a specific type of reads anyhow. - self.state = .unbuffering(.init(buffer: upgrading.buffer)) - return .fireErrorCaughtAndStartUnbuffering(error) } case .finished: @@ -317,7 +320,11 @@ struct NIOTypedHTTPServerUpgraderStateMachine { case .awaitingUpgrader(let awaitingUpgrader): switch result { case .success(.some((let upgrader, let responseHeaders, let proto))): - guard awaitingUpgrader.seenFirstRequest else { + if awaitingUpgrader.seenFirstRequest { + // We have seen the end of the request. So we can upgrade now. + self.state = .upgrading(.init(buffer: awaitingUpgrader.buffer)) + return .startUpgrading(upgrader: upgrader, responseHeaders: responseHeaders, proto: proto) + } else { // We have not yet seen the end so we have to wait until that happens self.state = .upgraderReady( .init( @@ -330,9 +337,6 @@ struct NIOTypedHTTPServerUpgraderStateMachine { ) return nil } - // We have seen the end of the request. So we can upgrade now. - self.state = .upgrading(.init(buffer: awaitingUpgrader.buffer)) - return .startUpgrading(upgrader: upgrader, responseHeaders: responseHeaders, proto: proto) case .success(.none): // There was no upgrader to handle the request. We just run the not upgrading @@ -341,12 +345,13 @@ struct NIOTypedHTTPServerUpgraderStateMachine { return .runNotUpgradingInitializer case .failure(let error): - guard !awaitingUpgrader.buffer.isEmpty else { + if !awaitingUpgrader.buffer.isEmpty { + self.state = .unbuffering(.init(buffer: awaitingUpgrader.buffer)) + return .fireErrorCaughtAndStartUnbuffering(error) + } else { self.state = .finished return .fireErrorCaughtAndRemoveHandler(error) } - self.state = .unbuffering(.init(buffer: awaitingUpgrader.buffer)) - return .fireErrorCaughtAndStartUnbuffering(error) } case .upgrading, .unbuffering, .finished: @@ -372,14 +377,15 @@ struct NIOTypedHTTPServerUpgraderStateMachine { case .unbuffering(var unbuffering): self.state = .modifying - guard let element = unbuffering.buffer.popFirst() else { + if let element = unbuffering.buffer.popFirst() { + self.state = .unbuffering(unbuffering) + + return .fireChannelRead(element) + } else { self.state = .finished return .fireChannelReadCompleteAndRemoveHandler } - self.state = .unbuffering(unbuffering) - - return .fireChannelRead(element) case .modifying: fatalError("Internal inconsistency in HTTPServerUpgradeStateMachine") diff --git a/Sources/NIOHTTP1Server/main.swift b/Sources/NIOHTTP1Server/main.swift index 313de3615b..728512fac8 100644 --- a/Sources/NIOHTTP1Server/main.swift +++ b/Sources/NIOHTTP1Server/main.swift @@ -17,10 +17,11 @@ import NIOPosix extension String { func chopPrefix(_ prefix: String) -> String? { - guard self.unicodeScalars.starts(with: prefix.unicodeScalars) else { + if self.unicodeScalars.starts(with: prefix.unicodeScalars) { + return String(self[self.index(self.startIndex, offsetBy: prefix.count)...]) + } else { return nil } - return String(self[self.index(self.startIndex, offsetBy: prefix.count)...]) } func containsDotDot() -> Bool { @@ -428,16 +429,17 @@ private final class HTTPHandler: ChannelInboundHandler { self.completeResponse(context, trailers: nil, promise: p) return p.futureResult }.flatMapError { error in - guard !responseStarted else { + if !responseStarted { + let response = httpResponseHead(request: request, status: .ok) + context.write(Self.wrapOutboundOut(.head(response)), promise: nil) + var buffer = context.channel.allocator.buffer(capacity: 100) + buffer.writeString("fail: \(error)") + context.write(Self.wrapOutboundOut(.body(.byteBuffer(buffer))), promise: nil) + self.state.responseComplete() + return context.writeAndFlush(Self.wrapOutboundOut(.end(nil))) + } else { return context.close() } - let response = httpResponseHead(request: request, status: .ok) - context.write(Self.wrapOutboundOut(.head(response)), promise: nil) - var buffer = context.channel.allocator.buffer(capacity: 100) - buffer.writeString("fail: \(error)") - context.write(Self.wrapOutboundOut(.body(.byteBuffer(buffer))), promise: nil) - self.state.responseComplete() - return context.writeAndFlush(Self.wrapOutboundOut(.end(nil))) }.whenComplete { (_: Result) in _ = try? file.close() } diff --git a/Sources/NIOPerformanceTester/ByteToMessageDecoderDecodeManySmallsBenchmark.swift b/Sources/NIOPerformanceTester/ByteToMessageDecoderDecodeManySmallsBenchmark.swift index c506e1cbfb..623a58f0c2 100644 --- a/Sources/NIOPerformanceTester/ByteToMessageDecoderDecodeManySmallsBenchmark.swift +++ b/Sources/NIOPerformanceTester/ByteToMessageDecoderDecodeManySmallsBenchmark.swift @@ -44,10 +44,11 @@ final class ByteToMessageDecoderDecodeManySmallsBenchmark: Benchmark { typealias InboundOut = Never func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - guard buffer.readSlice(length: 16) == nil else { + if buffer.readSlice(length: 16) == nil { + return .needMoreData + } else { return .continue } - return .needMoreData } } } diff --git a/Sources/NIOPerformanceTester/UDPBenchmark.swift b/Sources/NIOPerformanceTester/UDPBenchmark.swift index fc72312ffa..090b261bb8 100644 --- a/Sources/NIOPerformanceTester/UDPBenchmark.swift +++ b/Sources/NIOPerformanceTester/UDPBenchmark.swift @@ -167,12 +167,13 @@ extension UDPBenchmark { switch self.state { case .running(var running): running.responsesToRecieve &-= 1 - guard running.responsesToRecieve == 0, running.requestsToSend == 0 else { + if running.responsesToRecieve == 0, running.requestsToSend == 0 { + self.state = .stopped + return .finished(running.promise) + } else { self.state = .running(running) return .write } - self.state = .stopped - return .finished(running.promise) case .stopped: fatalError("Received too many messages") diff --git a/Sources/NIOPosix/BSDSocketAPIPosix.swift b/Sources/NIOPosix/BSDSocketAPIPosix.swift index 1519b8d1f9..852ab5b4d2 100644 --- a/Sources/NIOPosix/BSDSocketAPIPosix.swift +++ b/Sources/NIOPosix/BSDSocketAPIPosix.swift @@ -246,10 +246,11 @@ extension NIOBSDSocket { } // Only unlink the existing file if it is a socket - guard sb.st_mode & S_IFSOCK == S_IFSOCK else { + if sb.st_mode & S_IFSOCK == S_IFSOCK { + try Posix.unlink(pathname: path) + } else { throw UnixDomainSocketPathWrongType() } - try Posix.unlink(pathname: path) } catch let err as IOError { // If the filepath did not exist, we consider it cleaned up if err.errnoCode == ENOENT { diff --git a/Sources/NIOPosix/BaseSocketChannel+SocketOptionProvider.swift b/Sources/NIOPosix/BaseSocketChannel+SocketOptionProvider.swift index 859f6a5aa0..c50429abd3 100644 --- a/Sources/NIOPosix/BaseSocketChannel+SocketOptionProvider.swift +++ b/Sources/NIOPosix/BaseSocketChannel+SocketOptionProvider.swift @@ -33,16 +33,17 @@ extension BaseSocketChannel: SocketOptionProvider { name: NIOBSDSocket.Option, value: Value ) -> EventLoopFuture { - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + let promise = eventLoop.makePromise(of: Void.self) + executeAndComplete(promise) { + try setSocketOption0(level: level, name: name, value: value) + } + return promise.futureResult + } else { return eventLoop.submit { try self.setSocketOption0(level: level, name: name, value: value) } } - let promise = eventLoop.makePromise(of: Void.self) - executeAndComplete(promise) { - try setSocketOption0(level: level, name: name, value: value) - } - return promise.futureResult } #if !os(Windows) @@ -58,16 +59,17 @@ extension BaseSocketChannel: SocketOptionProvider { level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option ) -> EventLoopFuture { - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + let promise = eventLoop.makePromise(of: Value.self) + executeAndComplete(promise) { + try getSocketOption0(level: level, name: name) + } + return promise.futureResult + } else { return eventLoop.submit { try self.getSocketOption0(level: level, name: name) } } - let promise = eventLoop.makePromise(of: Value.self) - executeAndComplete(promise) { - try getSocketOption0(level: level, name: name) - } - return promise.futureResult } func setSocketOption0(level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option, value: Value) throws { diff --git a/Sources/NIOPosix/BaseSocketChannel.swift b/Sources/NIOPosix/BaseSocketChannel.swift index b6bac1301a..98cd75a57d 100644 --- a/Sources/NIOPosix/BaseSocketChannel.swift +++ b/Sources/NIOPosix/BaseSocketChannel.swift @@ -272,12 +272,13 @@ class BaseSocketChannel: SelectableChannel, Chan // This is called from arbitrary threads. internal var addressesCached: AddressCache { get { - guard self.eventLoop.inEventLoop else { + if self.eventLoop.inEventLoop { + return self._addressCache + } else { return self._offEventLoopLock.withLock { self._addressCache } } - return self._addressCache } set { self.eventLoop.preconditionInEventLoop() @@ -290,12 +291,13 @@ class BaseSocketChannel: SelectableChannel, Chan // This is called from arbitrary threads. private var bufferAllocatorCached: ByteBufferAllocator { get { - guard self.eventLoop.inEventLoop else { + if self.eventLoop.inEventLoop { + return self._bufferAllocatorCache + } else { return self._offEventLoopLock.withLock { self._bufferAllocatorCache } } - return self._bufferAllocatorCache } set { self.eventLoop.preconditionInEventLoop() @@ -606,12 +608,13 @@ class BaseSocketChannel: SelectableChannel, Chan } public final func setOption(_ option: Option, value: Option.Value) -> EventLoopFuture { - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + let promise = eventLoop.makePromise(of: Void.self) + executeAndComplete(promise) { try self.setOption0(option, value: value) } + return promise.futureResult + } else { return eventLoop.submit { try self.setOption0(option, value: value) } } - let promise = eventLoop.makePromise(of: Void.self) - executeAndComplete(promise) { try self.setOption0(option, value: value) } - return promise.futureResult } func setOption0(_ option: Option, value: Option.Value) throws { @@ -651,14 +654,15 @@ class BaseSocketChannel: SelectableChannel, Chan } public func getOption(_ option: Option) -> EventLoopFuture { - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + do { + return self.eventLoop.makeSucceededFuture(try self.getOption0(option)) + } catch { + return self.eventLoop.makeFailedFuture(error) + } + } else { return self.eventLoop.submit { try self.getOption0(option) } } - do { - return self.eventLoop.makeSucceededFuture(try self.getOption0(option)) - } catch { - return self.eventLoop.makeFailedFuture(error) - } } func getOption0(_ option: Option) throws -> Option.Value { diff --git a/Sources/NIOPosix/BaseStreamSocketChannel.swift b/Sources/NIOPosix/BaseStreamSocketChannel.swift index 9b6ef48ceb..605f551464 100644 --- a/Sources/NIOPosix/BaseStreamSocketChannel.swift +++ b/Sources/NIOPosix/BaseStreamSocketChannel.swift @@ -125,7 +125,22 @@ class BaseStreamSocketChannel: BaseSocketChannel // the end of the loop to not do an allocation when the loop exits. switch readResult { case .processed(let bytesRead): - guard bytesRead > 0 else { + if bytesRead > 0 { + self.recvBufferPool.record(actualReadBytes: bytesRead) + self.readPending = false + + assert(self.isActive) + self.pipeline.syncOperations.fireChannelRead(NIOAny(buffer)) + result = .some + + if buffer.writableBytes > 0 { + // If we did not fill the whole buffer with read(...) we should stop reading and wait until we get notified again. + // Otherwise chances are good that the next read(...) call will either read nothing or only a very small amount of data. + // Also this will allow us to call fireChannelReadComplete() which may give the user the chance to flush out all pending + // writes. + return result + } + } else { if self.inputShutdown { // We received a EOF because we called shutdown on the fd by ourself, unregister from the Selector and return self.readPending = false @@ -135,20 +150,6 @@ class BaseStreamSocketChannel: BaseSocketChannel // end-of-file throw ChannelError._eof } - self.recvBufferPool.record(actualReadBytes: bytesRead) - self.readPending = false - - assert(self.isActive) - self.pipeline.syncOperations.fireChannelRead(NIOAny(buffer)) - result = .some - - if buffer.writableBytes > 0 { - // If we did not fill the whole buffer with read(...) we should stop reading and wait until we get notified again. - // Otherwise chances are good that the next read(...) call will either read nothing or only a very small amount of data. - // Also this will allow us to call fireChannelReadComplete() which may give the user the chance to flush out all pending - // writes. - return result - } case .wouldBlock(let bytesRead): assert(bytesRead == 0) return result diff --git a/Sources/NIOPosix/Bootstrap.swift b/Sources/NIOPosix/Bootstrap.swift index ae1c1ef94e..6deaae1e1c 100644 --- a/Sources/NIOPosix/Bootstrap.swift +++ b/Sources/NIOPosix/Bootstrap.swift @@ -792,14 +792,15 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { private var protocolHandlers: Optional<@Sendable () -> [ChannelHandler]> private var _channelInitializer: ChannelInitializerCallback private var channelInitializer: ChannelInitializerCallback { - guard let protocolHandlers = self.protocolHandlers else { - return self._channelInitializer - } - let channelInitializer = _channelInitializer - return { channel in - channelInitializer(channel).flatMap { - channel.pipeline.addHandlers(protocolHandlers(), position: .first) + if let protocolHandlers = self.protocolHandlers { + let channelInitializer = _channelInitializer + return { channel in + channelInitializer(channel).flatMap { + channel.pipeline.addHandlers(protocolHandlers(), position: .first) + } } + } else { + return self._channelInitializer } } @usableFromInline @@ -1109,10 +1110,11 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { } } - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + return setupChannel() + } else { return eventLoop.flatSubmit { setupChannel() } } - return setupChannel() } private func initializeAndRegisterNewChannel( @@ -1141,12 +1143,13 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { func setupChannel() -> EventLoopFuture { eventLoop.assertInEventLoop() return channelOptions.applyAllChannelOptions(to: channel).flatMap { - guard let bindTarget = self.bindTarget else { + if let bindTarget = self.bindTarget { + return channel.bind(to: bindTarget).flatMap { + channelInitializer(channel) + } + } else { return channelInitializer(channel) } - return channel.bind(to: bindTarget).flatMap { - channelInitializer(channel) - } }.flatMap { eventLoop.assertInEventLoop() return channel.registerAndDoSynchronously(body) @@ -1158,12 +1161,13 @@ public final class ClientBootstrap: NIOClientTCPBootstrapProtocol { } } - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + return setupChannel() + } else { return eventLoop.flatSubmit { setupChannel() } } - return setupChannel() } } @@ -1411,15 +1415,16 @@ extension ClientBootstrap { channelOptions .applyAllChannelOptions(to: channel) .flatMap { - guard let bindTarget = bindTarget else { + if let bindTarget = bindTarget { + return + channel + .bind(to: bindTarget) + .flatMap { + channelInitializer(channel) + } + } else { return channelInitializer(channel) } - return - channel - .bind(to: bindTarget) - .flatMap { - channelInitializer(channel) - } }.flatMap { (result: ChannelInitializerResult) in eventLoop.assertInEventLoop() return registration(channel).map { @@ -1435,12 +1440,13 @@ extension ClientBootstrap { } } - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + return setupChannel() + } else { return eventLoop.flatSubmit { setupChannel() } } - return setupChannel() } } @@ -1712,12 +1718,13 @@ public final class DatagramBootstrap { } } - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + return setupChannel() + } else { return eventLoop.flatSubmit { setupChannel() } } - return setupChannel() } } @@ -2005,12 +2012,13 @@ extension DatagramBootstrap { } } - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + return setupChannel() + } else { return eventLoop.flatSubmit { setupChannel() } } - return setupChannel() } } @@ -2222,10 +2230,11 @@ public final class NIOPipeBootstrap { let eventLoop = self.group.next() let channelInitializer = self.channelInitializer return { channel in - guard let channelInitializer = channelInitializer else { + if let channelInitializer = channelInitializer { + return channelInitializer(channel).map { channel } + } else { return eventLoop.makeSucceededFuture(channel) } - return channelInitializer(channel).map { channel } } }() @@ -2446,12 +2455,13 @@ extension NIOPipeBootstrap { } } - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + return setupChannel() + } else { return eventLoop.flatSubmit { setupChannel() } } - return setupChannel() } } diff --git a/Sources/NIOPosix/GetaddrinfoResolver.swift b/Sources/NIOPosix/GetaddrinfoResolver.swift index 512dca2975..c5a7e5bfa7 100644 --- a/Sources/NIOPosix/GetaddrinfoResolver.swift +++ b/Sources/NIOPosix/GetaddrinfoResolver.swift @@ -104,13 +104,14 @@ internal class GetaddrinfoResolver: Resolver { if let offloadQueue = offloadQueueTSV.currentValue { return offloadQueue } else { - guard MultiThreadedEventLoopGroup.currentEventLoop != nil else { + if MultiThreadedEventLoopGroup.currentEventLoop != nil { + // Okay, we're on an SelectableEL thread. Let's stuff our queue into the thread local. + let offloadQueue = DispatchQueue(label: "io.swiftnio.GetaddrinfoResolver.offloadQueue") + offloadQueueTSV.currentValue = offloadQueue + return offloadQueue + } else { return DispatchQueue.global() } - // Okay, we're on an SelectableEL thread. Let's stuff our queue into the thread local. - let offloadQueue = DispatchQueue(label: "io.swiftnio.GetaddrinfoResolver.offloadQueue") - offloadQueueTSV.currentValue = offloadQueue - return offloadQueue } } diff --git a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift index 50d2d5324e..6e245b3862 100644 --- a/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift +++ b/Sources/NIOPosix/MultiThreadedEventLoopGroup.swift @@ -318,16 +318,17 @@ public final class MultiThreadedEventLoopGroup: EventLoopGroup { /// /// - returns: The `EventLoop`. public func any() -> EventLoop { - guard let loop = Self.currentSelectableEventLoop, + if let loop = Self.currentSelectableEventLoop, // We are on `loop`'s thread, so we may ask for the its parent group. loop.parentGroupCallableFromThisEventLoopOnly() === self - else { + { + // Nice, we can return this. + loop.assertInEventLoop() + return loop + } else { // Oh well, let's just vend the next one then. return self.next() } - // Nice, we can return this. - loop.assertInEventLoop() - return loop } /// Shut this `MultiThreadedEventLoopGroup` down which causes the `EventLoop`s and their associated threads to be @@ -542,10 +543,11 @@ extension ScheduledTask: CustomStringConvertible { extension ScheduledTask: Comparable { @usableFromInline static func < (lhs: ScheduledTask, rhs: ScheduledTask) -> Bool { - guard lhs.readyTime == rhs.readyTime else { + if lhs.readyTime == rhs.readyTime { + return lhs.id < rhs.id + } else { return lhs.readyTime < rhs.readyTime } - return lhs.id < rhs.id } @usableFromInline diff --git a/Sources/NIOPosix/NonBlockingFileIO.swift b/Sources/NIOPosix/NonBlockingFileIO.swift index 698354c679..2e1c3d565b 100644 --- a/Sources/NIOPosix/NonBlockingFileIO.swift +++ b/Sources/NIOPosix/NonBlockingFileIO.swift @@ -377,19 +377,20 @@ public struct NonBlockingFileIO: Sendable { while bytesRead < byteCount { let n = try buf.writeWithUnsafeMutableBytes(minimumWritableBytes: byteCount - bytesRead) { ptr in let res = try fileHandle.withUnsafeFileDescriptor { descriptor -> IOResult in - guard let offset = fromOffset else { + if let offset = fromOffset { + return try Posix.pread( + descriptor: descriptor, + pointer: ptr.baseAddress!, + size: byteCount - bytesRead, + offset: off_t(offset) + off_t(bytesRead) + ) + } else { return try Posix.read( descriptor: descriptor, pointer: ptr.baseAddress!, size: byteCount - bytesRead ) } - return try Posix.pread( - descriptor: descriptor, - pointer: ptr.baseAddress!, - size: byteCount - bytesRead, - offset: off_t(offset) + off_t(bytesRead) - ) } switch res { case .processed(let n): @@ -513,19 +514,20 @@ public struct NonBlockingFileIO: Sendable { let n = try buf.readWithUnsafeReadableBytes { ptr in precondition(ptr.count == byteCount - offsetAccumulator) let res: IOResult = try fileHandle.withUnsafeFileDescriptor { descriptor in - guard let toOffset = toOffset else { + if let toOffset = toOffset { + return try Posix.pwrite( + descriptor: descriptor, + pointer: ptr.baseAddress!, + size: byteCount - offsetAccumulator, + offset: off_t(toOffset + Int64(offsetAccumulator)) + ) + } else { return try Posix.write( descriptor: descriptor, pointer: ptr.baseAddress!, size: byteCount - offsetAccumulator ) } - return try Posix.pwrite( - descriptor: descriptor, - pointer: ptr.baseAddress!, - size: byteCount - offsetAccumulator, - offset: off_t(toOffset + Int64(offsetAccumulator)) - ) } switch res { case .processed(let n): diff --git a/Sources/NIOPosix/PendingDatagramWritesManager.swift b/Sources/NIOPosix/PendingDatagramWritesManager.swift index 7120fbaf2b..31365ca987 100644 --- a/Sources/NIOPosix/PendingDatagramWritesManager.swift +++ b/Sources/NIOPosix/PendingDatagramWritesManager.swift @@ -91,11 +91,12 @@ private func doPendingDatagramWriteVectorOperation( // plausible given that a similar limit exists for TCP. For now we assume it's present // in UDP until I can do some research to validate the existence of this limit. guard Socket.writevLimitBytes - toWrite >= p.data.readableBytes else { - guard c == 0 else { + if c == 0 { + // The first buffer is larger than the writev limit. Let's throw, and fall back to linear processing. + throw IOError(errnoCode: EMSGSIZE, reason: "synthetic error for overlarge write") + } else { break } - // The first buffer is larger than the writev limit. Let's throw, and fall back to linear processing. - throw IOError(errnoCode: EMSGSIZE, reason: "synthetic error for overlarge write") } // Must not write more than writevLimitIOVectors in one go @@ -208,14 +209,15 @@ private struct PendingDatagramWritesState { /// Check if there are no outstanding writes. public var isEmpty: Bool { - guard self.pendingWrites.isEmpty else { + if self.pendingWrites.isEmpty { + assert(self.chunks == 0) + assert(self.bytes == 0) + assert(!self.pendingWrites.hasMark) + return true + } else { assert(self.chunks > 0 && self.bytes >= 0) return false } - assert(self.chunks == 0) - assert(self.bytes == 0) - assert(!self.pendingWrites.hasMark) - return true } /// Add a new write and optionally the corresponding promise to the list of outstanding writes. @@ -250,10 +252,11 @@ private struct PendingDatagramWritesState { ) -> (DatagramWritePromiseFiller?, OneWriteOperationResult) { switch data { case .processed(let written): - guard let messages = messages else { + if let messages = messages { + return didVectorWrite(written: written, messages: messages) + } else { return didScalarWrite(written: written) } - return didVectorWrite(written: written, messages: messages) case .wouldBlock: return (nil, .wouldBlock) } @@ -472,7 +475,22 @@ final class PendingDatagramWritesManager: PendingWritesManager { /// - warning: If the socket is connected, then the `envelope.remoteAddress` _must_ match the /// address of the connected peer, otherwise this function will throw a fatal error. func add(envelope: AddressedEnvelope, promise: EventLoopPromise?) -> Bool { - guard let remoteAddress = self.state.remoteAddress else { + if let remoteAddress = self.state.remoteAddress { + precondition( + envelope.remoteAddress == remoteAddress, + """ + Remote address of AddressedEnvelope does not match remote address of connected socket. + """ + ) + return self.add( + PendingDatagramWrite( + data: envelope.data, + promise: promise, + address: nil, + metadata: envelope.metadata + ) + ) + } else { return self.add( PendingDatagramWrite( data: envelope.data, @@ -482,20 +500,6 @@ final class PendingDatagramWritesManager: PendingWritesManager { ) ) } - precondition( - envelope.remoteAddress == remoteAddress, - """ - Remote address of AddressedEnvelope does not match remote address of connected socket. - """ - ) - return self.add( - PendingDatagramWrite( - data: envelope.data, - promise: promise, - address: nil, - metadata: envelope.metadata - ) - ) } /// Add a pending write, without an `AddressedEnvelope`, on a connected socket. diff --git a/Sources/NIOPosix/PendingWritesManager.swift b/Sources/NIOPosix/PendingWritesManager.swift index bcd787a0a7..10c22cd204 100644 --- a/Sources/NIOPosix/PendingWritesManager.swift +++ b/Sources/NIOPosix/PendingWritesManager.swift @@ -156,13 +156,14 @@ private struct PendingStreamWritesState { /// Check if there are no outstanding writes. public var isEmpty: Bool { - guard self.pendingWrites.isEmpty else { + if self.pendingWrites.isEmpty { + assert(self.bytes == 0) + assert(!self.pendingWrites.hasMark) + return true + } else { assert(self.bytes >= 0) return false } - assert(self.bytes == 0) - assert(!self.pendingWrites.hasMark) - return true } /// Add a new write and optionally the corresponding promise to the list of outstanding writes. @@ -210,22 +211,23 @@ private struct PendingStreamWritesState { var unaccountedWrites = written for _ in 0..= headItemReadableBytes else { + if unaccountedWrites >= headItemReadableBytes { + unaccountedWrites -= headItemReadableBytes + // we wrote at least the whole head item, so drop it and succeed the promise + if let promise = self.fullyWrittenFirst() { + if let p = promise0 { + p.futureResult.cascade(to: promise) + } else { + promise0 = promise + } + } + } else { // we could only write a part of the head item, so don't drop it but remember what we wrote self.partiallyWrittenFirst(bytes: unaccountedWrites) // may try again depending on the writeSpinCount return (promise0, .writtenPartially) } - unaccountedWrites -= headItemReadableBytes - // we wrote at least the whole head item, so drop it and succeed the promise - if let promise = self.fullyWrittenFirst() { - if let p = promise0 { - p.futureResult.cascade(to: promise) - } else { - promise0 = promise - } - } } assert( unaccountedWrites == 0, diff --git a/Sources/NIOPosix/Pool.swift b/Sources/NIOPosix/Pool.swift index 870ff1d8f1..e46f57537b 100644 --- a/Sources/NIOPosix/Pool.swift +++ b/Sources/NIOPosix/Pool.swift @@ -33,10 +33,11 @@ class Pool { } func get() -> Element { - guard elements.isEmpty else { + if elements.isEmpty { + return Element() + } else { return elements.removeLast() } - return Element() } func put(_ e: Element) { diff --git a/Sources/NIOPosix/PooledRecvBufferAllocator.swift b/Sources/NIOPosix/PooledRecvBufferAllocator.swift index 4c648780f1..f34621cdc3 100644 --- a/Sources/NIOPosix/PooledRecvBufferAllocator.swift +++ b/Sources/NIOPosix/PooledRecvBufferAllocator.swift @@ -47,13 +47,14 @@ internal struct PooledRecvBufferAllocator { /// Returns the number of buffers in the pool. var count: Int { - guard self.buffer == nil else { + if self.buffer == nil { + // Empty or switched to `buffers` for storage. + return self.buffers.count + } else { // `buffer` is non-nil; `buffers` must be empty and the count must be 1. assert(self.buffers.isEmpty) return 1 } - // Empty or switched to `buffers` for storage. - return self.buffers.count } /// Update the capacity of the underlying buffer pool. @@ -92,12 +93,13 @@ internal struct PooledRecvBufferAllocator { _ body: (inout ByteBuffer) throws -> Result ) rethrows -> (ByteBuffer, Result) { // Reuse an existing buffer if we can do so without CoWing. - guard let bufferAndResult = try self.reuseExistingBuffer(body) else { + if let bufferAndResult = try self.reuseExistingBuffer(body) { + return bufferAndResult + } else { // No available buffers or the allocator does not offer up buffer sizes; directly // allocate a new one. return try self.allocateNewBuffer(using: allocator, body) } - return bufferAndResult } private mutating func reuseExistingBuffer( @@ -144,16 +146,17 @@ internal struct PooledRecvBufferAllocator { // We have a stored buffer, either: // 1. We have capacity to add more and use `buffers` for storage, or // 2. Our capacity is 1; we can't use `buffers` for storage. - guard self.capacity > 1 else { + if self.capacity > 1 { + self.buffer = nil + self.buffers.reserveCapacity(self.capacity) + self.buffers.append(buffer) + self.buffers.append(newBuffer) + self.lastUsedIndex = self.buffers.index(before: self.buffers.endIndex) + return try self.modifyBuffer(atIndex: self.lastUsedIndex, body) + } else { let result = try body(&newBuffer) return (newBuffer, result) } - self.buffer = nil - self.buffers.reserveCapacity(self.capacity) - self.buffers.append(buffer) - self.buffers.append(newBuffer) - self.lastUsedIndex = self.buffers.index(before: self.buffers.endIndex) - return try self.modifyBuffer(atIndex: self.lastUsedIndex, body) } else { // There's no stored buffer which could be due to: // 1. this is the first buffer we allocate (i.e. buffers is empty, we already know diff --git a/Sources/NIOPosix/RawSocketBootstrap.swift b/Sources/NIOPosix/RawSocketBootstrap.swift index 65355587de..5f507fb5e9 100644 --- a/Sources/NIOPosix/RawSocketBootstrap.swift +++ b/Sources/NIOPosix/RawSocketBootstrap.swift @@ -196,12 +196,13 @@ public final class NIORawSocketBootstrap { } } - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + return setupChannel() + } else { return eventLoop.flatSubmit { setupChannel() } } - return setupChannel() } } @@ -359,12 +360,13 @@ extension NIORawSocketBootstrap { } } - guard eventLoop.inEventLoop else { + if eventLoop.inEventLoop { + return setupChannel() + } else { return eventLoop.flatSubmit { setupChannel() } } - return setupChannel() } } diff --git a/Sources/NIOPosix/SelectableEventLoop.swift b/Sources/NIOPosix/SelectableEventLoop.swift index 89884f0cf1..c8f9204cfb 100644 --- a/Sources/NIOPosix/SelectableEventLoop.swift +++ b/Sources/NIOPosix/SelectableEventLoop.swift @@ -404,13 +404,14 @@ internal final class SelectableEventLoop: EventLoop { self._immediateTasks.append(task) } - guard self._pendingTaskPop == false else { + if self._pendingTaskPop == false { + // Our job to wake the selector. + self._pendingTaskPop = true + return true + } else { // There is already an event-loop-tick scheduled, we don't need to wake the selector. return false } - // Our job to wake the selector. - self._pendingTaskPop = true - return true } } @@ -474,11 +475,12 @@ internal final class SelectableEventLoop: EventLoop { } let nextReady = deadline.readyIn(.now()) - guard nextReady <= .nanoseconds(0) else { + if nextReady <= .nanoseconds(0) { + // Something is ready to be processed just do a non-blocking select of events. + return .now + } else { return .blockUntilTimeout(nextReady) } - // Something is ready to be processed just do a non-blocking select of events. - return .now } private func run(_ task: UnderlyingTask) { @@ -788,11 +790,12 @@ internal final class SelectableEventLoop: EventLoop { } } else { let goAhead = self.externalStateLock.withLock { () -> Bool in - guard self.externalState == .open else { + if self.externalState == .open { + self.externalState = .closing + return true + } else { return false } - self.externalState = .closing - return true } guard goAhead else { queue.async { diff --git a/Sources/NIOPosix/SelectorGeneric.swift b/Sources/NIOPosix/SelectorGeneric.swift index 689d198705..f865021bd7 100644 --- a/Sources/NIOPosix/SelectorGeneric.swift +++ b/Sources/NIOPosix/SelectorGeneric.swift @@ -23,12 +23,13 @@ internal enum SelectorLifecycleState { extension Optional { internal func withUnsafeOptionalPointer(_ body: (UnsafePointer?) throws -> T) rethrows -> T { - guard var this = self else { + if var this = self { + return try withUnsafePointer(to: &this) { x in + try body(x) + } + } else { return try body(nil) } - return try withUnsafePointer(to: &this) { x in - try body(x) - } } } @@ -343,12 +344,13 @@ extension Selector: CustomStringConvertible { "Selector { descriptor = \(self.selectorFD) }" } - guard NIOThread.current == self.myThread else { + if NIOThread.current == self.myThread { + return makeDescription() + } else { return self.externalSelectorFDLock.withLock { makeDescription() } } - return makeDescription() } } @@ -401,10 +403,11 @@ extension Selector where R == NIORegistration { } }.map { future in future.flatMapErrorThrowing { error in - guard let error = error as? ChannelError, error == .alreadyClosed else { + if let error = error as? ChannelError, error == .alreadyClosed { + return () + } else { throw error } - return () } } diff --git a/Sources/NIOPosix/SocketChannel.swift b/Sources/NIOPosix/SocketChannel.swift index 2fea00a213..a299a8ec75 100644 --- a/Sources/NIOPosix/SocketChannel.swift +++ b/Sources/NIOPosix/SocketChannel.swift @@ -336,23 +336,24 @@ final class ServerSocketChannel: BaseSocketChannel { guard self.isOpen else { throw ChannelError._eof } - guard let accepted = try self.socket.accept(setNonBlocking: true) else { + if let accepted = try self.socket.accept(setNonBlocking: true) { + readPending = false + result = .some + do { + let chan = try SocketChannel( + socket: accepted, + parent: self, + eventLoop: group.next() as! SelectableEventLoop + ) + assert(self.isActive) + self.pipeline.syncOperations.fireChannelRead(NIOAny(chan)) + } catch { + try? accepted.close() + throw error + } + } else { break } - readPending = false - result = .some - do { - let chan = try SocketChannel( - socket: accepted, - parent: self, - eventLoop: group.next() as! SelectableEventLoop - ) - assert(self.isActive) - self.pipeline.syncOperations.fireChannelRead(NIOAny(chan)) - } catch { - try? accepted.close() - throw error - } } return result } diff --git a/Sources/NIOPosix/System.swift b/Sources/NIOPosix/System.swift index 599f6fe0b1..b9f6b9710e 100644 --- a/Sources/NIOPosix/System.swift +++ b/Sources/NIOPosix/System.swift @@ -581,10 +581,11 @@ internal enum Posix { sysAccept(descriptor, addr, len) } - guard case .processed(let fd) = result else { + if case .processed(let fd) = result { + return fd + } else { return nil } - return fd } @inline(never) diff --git a/Sources/NIOTCPEchoClient/Client.swift b/Sources/NIOTCPEchoClient/Client.swift index a385d0e548..e624a78667 100644 --- a/Sources/NIOTCPEchoClient/Client.swift +++ b/Sources/NIOTCPEchoClient/Client.swift @@ -97,13 +97,14 @@ private final class NewlineDelimiterCoder: ByteToMessageDecoder, MessageToByteEn func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { let readableBytes = buffer.readableBytesView - guard let firstLine = readableBytes.firstIndex(of: self.newLine).map({ readableBytes[..<$0] }) else { + if let firstLine = readableBytes.firstIndex(of: self.newLine).map({ readableBytes[..<$0] }) { + buffer.moveReaderIndex(forwardBy: firstLine.count + 1) + // Fire a read without a newline + context.fireChannelRead(Self.wrapInboundOut(String(buffer: ByteBuffer(firstLine)))) + return .continue + } else { return .needMoreData } - buffer.moveReaderIndex(forwardBy: firstLine.count + 1) - // Fire a read without a newline - context.fireChannelRead(Self.wrapInboundOut(String(buffer: ByteBuffer(firstLine)))) - return .continue } func encode(data: String, out: inout ByteBuffer) throws { diff --git a/Sources/NIOTCPEchoServer/Server.swift b/Sources/NIOTCPEchoServer/Server.swift index e70fbcce1e..451d687b89 100644 --- a/Sources/NIOTCPEchoServer/Server.swift +++ b/Sources/NIOTCPEchoServer/Server.swift @@ -107,13 +107,14 @@ private final class NewlineDelimiterCoder: ByteToMessageDecoder, MessageToByteEn func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { let readableBytes = buffer.readableBytesView - guard let firstLine = readableBytes.firstIndex(of: self.newLine).map({ readableBytes[..<$0] }) else { + if let firstLine = readableBytes.firstIndex(of: self.newLine).map({ readableBytes[..<$0] }) { + buffer.moveReaderIndex(forwardBy: firstLine.count + 1) + // Fire a read without a newline + context.fireChannelRead(Self.wrapInboundOut(String(buffer: ByteBuffer(firstLine)))) + return .continue + } else { return .needMoreData } - buffer.moveReaderIndex(forwardBy: firstLine.count + 1) - // Fire a read without a newline - context.fireChannelRead(Self.wrapInboundOut(String(buffer: ByteBuffer(firstLine)))) - return .continue } func encode(data: String, out: inout ByteBuffer) throws { diff --git a/Sources/NIOTLS/ProtocolNegotiationHandlerStateMachine.swift b/Sources/NIOTLS/ProtocolNegotiationHandlerStateMachine.swift index 4fe329d46c..4dca2ae527 100644 --- a/Sources/NIOTLS/ProtocolNegotiationHandlerStateMachine.swift +++ b/Sources/NIOTLS/ProtocolNegotiationHandlerStateMachine.swift @@ -53,19 +53,20 @@ struct ProtocolNegotiationHandlerStateMachine { @inlinable mutating func userInboundEventTriggered(event: Any) -> UserInboundEventTriggeredAction { - guard case .handshakeCompleted(let negotiated) = event as? TLSUserEvent else { - return .fireUserInboundEventTriggered - } - switch self.state { - case .initial: - self.state = .waitingForUser(buffer: .init()) - - return .invokeUserClosure(.init(negotiated: negotiated)) - case .waitingForUser, .unbuffering: - preconditionFailure("Unexpectedly received two TLSUserEvents") - - case .finished: - // This is weird but we can tolerate it and just forward the event + if case .handshakeCompleted(let negotiated) = event as? TLSUserEvent { + switch self.state { + case .initial: + self.state = .waitingForUser(buffer: .init()) + + return .invokeUserClosure(.init(negotiated: negotiated)) + case .waitingForUser, .unbuffering: + preconditionFailure("Unexpectedly received two TLSUserEvents") + + case .finished: + // This is weird but we can tolerate it and just forward the event + return .fireUserInboundEventTriggered + } + } else { return .fireUserInboundEventTriggered } } @@ -113,20 +114,22 @@ struct ProtocolNegotiationHandlerStateMachine { switch result { case .success(let value): - guard !buffer.isEmpty else { + if !buffer.isEmpty { + self.state = .unbuffering(buffer: buffer) + return .startUnbuffering(value) + } else { self.state = .finished return .removeHandler(value) } - self.state = .unbuffering(buffer: buffer) - return .startUnbuffering(value) case .failure(let error): - guard !buffer.isEmpty else { + if !buffer.isEmpty { + self.state = .unbuffering(buffer: buffer) + return .fireErrorCaughtAndStartUnbuffering(error) + } else { self.state = .finished return .fireErrorCaughtAndRemoveHandler(error) } - self.state = .unbuffering(buffer: buffer) - return .fireErrorCaughtAndStartUnbuffering(error) } case .unbuffering: @@ -151,14 +154,15 @@ struct ProtocolNegotiationHandlerStateMachine { preconditionFailure("Invalid state \(self.state)") case .unbuffering(var buffer): - guard let element = buffer.popFirst() else { + if let element = buffer.popFirst() { + self.state = .unbuffering(buffer: buffer) + + return .fireChannelRead(element) + } else { self.state = .finished return .fireChannelReadCompleteAndRemoveHandler } - self.state = .unbuffering(buffer: buffer) - - return .fireChannelRead(element) } } diff --git a/Sources/NIOTLS/SNIHandler.swift b/Sources/NIOTLS/SNIHandler.swift index a9c71fdc97..18efcac439 100644 --- a/Sources/NIOTLS/SNIHandler.swift +++ b/Sources/NIOTLS/SNIHandler.swift @@ -404,10 +404,11 @@ public final class SNIHandler: ByteToMessageDecoder { } return UnsafeRawBufferPointer(rebasing: ptr.prefix(nameLength)).decodeStringValidatingASCII() } - guard let hostname = hostname else { + if let hostname = hostname { + return hostname + } else { throw InternalSNIErrors.invalidRecord } - return hostname } return nil } diff --git a/Sources/NIOTestUtils/NIOHTTP1TestServer.swift b/Sources/NIOTestUtils/NIOHTTP1TestServer.swift index 3d180dfaee..b4326f5c58 100644 --- a/Sources/NIOTestUtils/NIOHTTP1TestServer.swift +++ b/Sources/NIOTestUtils/NIOHTTP1TestServer.swift @@ -218,10 +218,11 @@ public final class NIOHTTP1TestServer { return } channel.pipeline.configureHTTPServerPipeline().flatMap { - guard self.aggregateBody else { + if self.aggregateBody { + return channel.pipeline.addHandler(AggregateBodyHandler()) + } else { return self.eventLoop.makeSucceededVoidFuture() } - return channel.pipeline.addHandler(AggregateBodyHandler()) }.flatMap { channel.pipeline.addHandler(WebServerHandler(webServer: self)) }.whenSuccess { @@ -305,10 +306,11 @@ extension NIOHTTP1TestServer { public func writeOutbound(_ data: HTTPServerResponsePart) throws { self.eventLoop.assertNotInEventLoop() try self.eventLoop.flatSubmit { () -> EventLoopFuture in - guard let channel = self.currentClientChannel else { + if let channel = self.currentClientChannel { + return channel.writeAndFlush(data) + } else { return self.eventLoop.makeFailedFuture(ChannelError.ioOnClosedChannel) } - return channel.writeAndFlush(data) }.wait() } diff --git a/Sources/_NIODataStructures/Heap.swift b/Sources/_NIODataStructures/Heap.swift index 7cdb510320..9af0c0622b 100644 --- a/Sources/_NIODataStructures/Heap.swift +++ b/Sources/_NIODataStructures/Heap.swift @@ -114,11 +114,12 @@ internal struct Heap { @discardableResult @inlinable internal mutating func remove(value: Element) -> Bool { - guard let idx = self.storage.firstIndex(of: value) else { + if let idx = self.storage.firstIndex(of: value) { + self._remove(index: idx) + return true + } else { return false } - self._remove(index: idx) - return true } @inlinable diff --git a/Tests/NIOCoreTests/SingleStepByteToMessageDecoderTest.swift b/Tests/NIOCoreTests/SingleStepByteToMessageDecoderTest.swift index 77757c55fa..60921984db 100644 --- a/Tests/NIOCoreTests/SingleStepByteToMessageDecoderTest.swift +++ b/Tests/NIOCoreTests/SingleStepByteToMessageDecoderTest.swift @@ -284,22 +284,24 @@ public final class NIOSingleStepByteToMessageDecoderTest: XCTestCase { var state: Int = 1 mutating func decode(buffer: inout ByteBuffer) throws -> InboundOut? { - guard buffer.readSlice(length: self.state) != nil else { + if buffer.readSlice(length: self.state) != nil { + defer { + self.state += 1 + } + return self.state + } else { return nil } - defer { - self.state += 1 - } - return self.state } mutating func decodeLast(buffer: inout ByteBuffer, seenEOF: Bool) throws -> InboundOut? { XCTAssertTrue(seenEOF) - guard self.state > 0 else { + if self.state > 0 { + self.state = 0 + return buffer.readableBytes * -1 + } else { return nil } - self.state = 0 - return buffer.readableBytes * -1 } } let allocator = ByteBufferAllocator() diff --git a/Tests/NIOCoreTests/XCTest+Extensions.swift b/Tests/NIOCoreTests/XCTest+Extensions.swift index ab32218d4b..1717542d9e 100644 --- a/Tests/NIOCoreTests/XCTest+Extensions.swift +++ b/Tests/NIOCoreTests/XCTest+Extensions.swift @@ -47,10 +47,11 @@ func assertNoThrowWithValue( return try body() } catch { XCTFail("\(message.map { $0 + ": " } ?? "")unexpected error \(error) thrown", file: (file), line: line) - guard let defaultValue = defaultValue else { + if let defaultValue = defaultValue { + return defaultValue + } else { throw error } - return defaultValue } } @@ -79,10 +80,11 @@ private var temporaryDirectory: String { #if os(Linux) return "/tmp" #else - guard #available(macOS 10.12, iOS 10, tvOS 10, watchOS 3, *) else { + if #available(macOS 10.12, iOS 10, tvOS 10, watchOS 3, *) { + return FileManager.default.temporaryDirectory.path + } else { return "/tmp" } - return FileManager.default.temporaryDirectory.path #endif // os #endif // targetEnvironment } diff --git a/Tests/NIODataStructuresTests/HeapTests.swift b/Tests/NIODataStructuresTests/HeapTests.swift index d4194b1035..186ae4ad11 100644 --- a/Tests/NIODataStructuresTests/HeapTests.swift +++ b/Tests/NIODataStructuresTests/HeapTests.swift @@ -120,7 +120,9 @@ extension Heap { func checkHeapProperty(index: Int) -> Bool { let li = self.leftIndex(index) let ri = self.rightIndex(index) - guard index >= self.storage.count else { + if index >= self.storage.count { + return true + } else { let me = self.storage[index] var lCond = true var rCond = true @@ -134,7 +136,6 @@ extension Heap { } return lCond && rCond && checkHeapProperty(index: li) && checkHeapProperty(index: ri) } - return true } return checkHeapProperty(index: 0) } diff --git a/Tests/NIOEmbeddedTests/TestUtils.swift b/Tests/NIOEmbeddedTests/TestUtils.swift index e5cab88165..02fe152e7d 100644 --- a/Tests/NIOEmbeddedTests/TestUtils.swift +++ b/Tests/NIOEmbeddedTests/TestUtils.swift @@ -41,7 +41,15 @@ func assert( extension EventLoopFuture { var isFulfilled: Bool { - guard self.eventLoop.inEventLoop else { + if self.eventLoop.inEventLoop { + // Easy, we're on the EventLoop. Let's just use our knowledge that we run completed future callbacks + // immediately. + var fulfilled = false + self.whenComplete { _ in + fulfilled = true + } + return fulfilled + } else { let lock = NIOLock() let group = DispatchGroup() var fulfilled = false // protected by lock @@ -57,13 +65,6 @@ extension EventLoopFuture { group.wait() // this is very nasty but this is for tests only, so... return lock.withLock { fulfilled } } - // Easy, we're on the EventLoop. Let's just use our knowledge that we run completed future callbacks - // immediately. - var fulfilled = false - self.whenComplete { _ in - fulfilled = true - } - return fulfilled } } diff --git a/Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift b/Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift index 11c5557d55..673f119f78 100644 --- a/Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift +++ b/Tests/NIOFileSystemIntegrationTests/FileSystemTests.swift @@ -34,11 +34,12 @@ extension FileSystem { _ function: String = #function, inTemporaryDirectory: Bool = true ) async throws -> FilePath { - guard inTemporaryDirectory else { + if inTemporaryDirectory { + let directory = try await self.temporaryDirectory + return self.temporaryFilePath(function, inDirectory: directory) + } else { return self.temporaryFilePath(function, inDirectory: nil) } - let directory = try await self.temporaryDirectory - return self.temporaryFilePath(function, inDirectory: directory) } func temporaryFilePath( @@ -50,10 +51,11 @@ extension FileSystem { let random = UInt32.random(in: .min ... .max) let fileName = "\(functionName)-\(random)" - guard let directory = directory else { + if let directory = directory { + return directory.appending(fileName) + } else { return FilePath(fileName) } - return directory.appending(fileName) } } diff --git a/Tests/NIOFileSystemTests/Internal/SyscallTests.swift b/Tests/NIOFileSystemTests/Internal/SyscallTests.swift index 6749e3816d..e971c5f943 100644 --- a/Tests/NIOFileSystemTests/Internal/SyscallTests.swift +++ b/Tests/NIOFileSystemTests/Internal/SyscallTests.swift @@ -423,13 +423,14 @@ final class SyscallTests: XCTestCase { var shouldInterrupt = true let r2: Result = valueOrErrno(retryOnInterrupt: true) { - guard shouldInterrupt else { + if shouldInterrupt { + shouldInterrupt = false + Errno._current = .interrupted + return -1 + } else { Errno._current = .permissionDenied return -1 } - shouldInterrupt = false - Errno._current = .interrupted - return -1 } XCTAssertFalse(shouldInterrupt) XCTAssertEqual(r2, .failure(.permissionDenied)) @@ -450,13 +451,14 @@ final class SyscallTests: XCTestCase { var shouldInterrupt = true let r2: Result = nothingOrErrno(retryOnInterrupt: true) { - guard shouldInterrupt else { + if shouldInterrupt { + shouldInterrupt = false + Errno._current = .interrupted + return -1 + } else { Errno._current = .permissionDenied return -1 } - shouldInterrupt = false - Errno._current = .interrupted - return -1 } XCTAssertFalse(shouldInterrupt) XCTAssertThrowsError(try r2.get()) { error in @@ -476,12 +478,13 @@ final class SyscallTests: XCTestCase { var shouldInterrupt = true let r2: Result = optionalValueOrErrno(retryOnInterrupt: true) { - guard shouldInterrupt else { + if shouldInterrupt { + shouldInterrupt = false + Errno._current = .interrupted + return nil + } else { return "foo" } - shouldInterrupt = false - Errno._current = .interrupted - return nil } XCTAssertFalse(shouldInterrupt) XCTAssertEqual(r2, .success("foo")) diff --git a/Tests/NIOHTTP1Tests/HTTPServerUpgradeTests.swift b/Tests/NIOHTTP1Tests/HTTPServerUpgradeTests.swift index fe1ba837f9..343b6b9138 100644 --- a/Tests/NIOHTTP1Tests/HTTPServerUpgradeTests.swift +++ b/Tests/NIOHTTP1Tests/HTTPServerUpgradeTests.swift @@ -1791,13 +1791,14 @@ final class TypedHTTPServerUpgradeTestCase: HTTPServerUpgradeTestCase { configuration: configuration ) .flatMap { result in - guard result else { + if result { + return channel.pipeline.context(handlerType: NIOTypedHTTPServerUpgradeHandler.self) + .map { + upgradeCompletionHandler($0) + } + } else { return channel.eventLoop.makeSucceededVoidFuture() } - return channel.pipeline.context(handlerType: NIOTypedHTTPServerUpgradeHandler.self) - .map { - upgradeCompletionHandler($0) - } } } .flatMap { _ in diff --git a/Tests/NIOHTTP1Tests/HTTPTest.swift b/Tests/NIOHTTP1Tests/HTTPTest.swift index 88ab8c0860..b1cf126e80 100644 --- a/Tests/NIOHTTP1Tests/HTTPTest.swift +++ b/Tests/NIOHTTP1Tests/HTTPTest.swift @@ -124,16 +124,17 @@ class HTTPTest: XCTestCase { XCTAssertNoThrow(try EventLoopFuture.andAllSucceed(writeFutures, on: channel.eventLoop).wait()) XCTAssertEqual(2 * expecteds.count, step) - guard body != nil else { + if body != nil { + XCTAssertGreaterThan(allBodyDatas.count, 0) + let firstBodyData = allBodyDatas[0] + for bodyData in allBodyDatas { + XCTAssertEqual(firstBodyData, bodyData) + } + return String(decoding: firstBodyData, as: Unicode.UTF8.self) + } else { XCTAssertEqual(0, allBodyDatas.count, "left with \(allBodyDatas)") return nil } - XCTAssertGreaterThan(allBodyDatas.count, 0) - let firstBodyData = allBodyDatas[0] - for bodyData in allBodyDatas { - XCTAssertEqual(firstBodyData, bodyData) - } - return String(decoding: firstBodyData, as: Unicode.UTF8.self) } // send all bytes in one go diff --git a/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift b/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift index 529b52cf69..b1fc060feb 100644 --- a/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift +++ b/Tests/NIOPosixTests/AsyncChannelBootstrapTests.swift @@ -46,19 +46,20 @@ private final class LineDelimiterCoder: ByteToMessageDecoder, MessageToByteEncod func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { let readable = buffer.withUnsafeReadableBytes { $0.firstIndex(of: self.newLine) } if let readable = readable { - guard let id = self.inboundID else { + if let id = self.inboundID { + let data = buffer.readSlice(length: readable - 1)! + let inboundID = buffer.readInteger(as: UInt8.self)! + buffer.moveReaderIndex(forwardBy: 1) + + if id == inboundID { + context.fireChannelRead(Self.wrapInboundOut(data)) + } + return .continue + } else { context.fireChannelRead(Self.wrapInboundOut(buffer.readSlice(length: readable)!)) buffer.moveReaderIndex(forwardBy: 1) return .continue } - let data = buffer.readSlice(length: readable - 1)! - let inboundID = buffer.readInteger(as: UInt8.self)! - buffer.moveReaderIndex(forwardBy: 1) - - if id == inboundID { - context.fireChannelRead(Self.wrapInboundOut(data)) - } - return .continue } return .needMoreData } diff --git a/Tests/NIOPosixTests/ChannelTests.swift b/Tests/NIOPosixTests/ChannelTests.swift index 0fce149100..8b86e834a4 100644 --- a/Tests/NIOPosixTests/ChannelTests.swift +++ b/Tests/NIOPosixTests/ChannelTests.swift @@ -287,32 +287,53 @@ public final class ChannelTests: XCTestCase { singleState += 1 everythingState += 1 } - guard let expected = expectedSingleWritabilities else { + if let expected = expectedSingleWritabilities { + if expected.count > singleState { + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + expected[singleState], + buf.count, + "in single write \(singleState) (overall \(everythingState)), \(expected[singleState]) bytes expected but \(buf.count) actual" + ) + return returns[everythingState] + } else { + XCTFail( + "single write call \(singleState) but less than \(expected.count) expected", + file: (file), + line: line + ) + return IOResult.wouldBlock(-1 * (everythingState + 1)) + } + } else { XCTFail("single write called on \(buf) but no single writes expected", file: (file), line: line) return IOResult.wouldBlock(-1 * (everythingState + 1)) } - guard expected.count > singleState else { - XCTFail( - "single write call \(singleState) but less than \(expected.count) expected", - file: (file), - line: line - ) - return IOResult.wouldBlock(-1 * (everythingState + 1)) - } - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - expected[singleState], - buf.count, - "in single write \(singleState) (overall \(everythingState)), \(expected[singleState]) bytes expected but \(buf.count) actual" - ) - return returns[everythingState] }, vectorBufferWriteOperation: { ptrs in defer { multiState += 1 everythingState += 1 } - guard let expected = expectedVectorWritabilities else { + if let expected = expectedVectorWritabilities { + if expected.count > multiState { + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + expected[multiState], + ptrs.map { numericCast($0.iov_len) }, + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.iov_len }) actual", + file: (file), + line: line + ) + return returns[everythingState] + } else { + XCTFail( + "vector write call \(multiState) but less than \(expected.count) expected", + file: (file), + line: line + ) + return IOResult.wouldBlock(-1 * (everythingState + 1)) + } + } else { XCTFail( "vector write called on \(ptrs) but no vector writes expected", file: (file), @@ -320,23 +341,6 @@ public final class ChannelTests: XCTestCase { ) return IOResult.wouldBlock(-1 * (everythingState + 1)) } - guard expected.count > multiState else { - XCTFail( - "vector write call \(multiState) but less than \(expected.count) expected", - file: (file), - line: line - ) - return IOResult.wouldBlock(-1 * (everythingState + 1)) - } - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - expected[multiState], - ptrs.map { numericCast($0.iov_len) }, - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.iov_len }) actual", - file: (file), - line: line - ) - return returns[everythingState] }, scalarFileWriteOperation: { _, start, end in defer { @@ -352,7 +356,24 @@ public final class ChannelTests: XCTestCase { return IOResult.wouldBlock(-1 * (everythingState + 1)) } - guard expected.count > fileState else { + if expected.count > fileState { + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + expected[fileState].0, + start, + "in file write \(fileState) (overall \(everythingState)), \(expected[fileState].0) expected as start index but \(start) actual", + file: (file), + line: line + ) + XCTAssertEqual( + expected[fileState].1, + end, + "in file write \(fileState) (overall \(everythingState)), \(expected[fileState].1) expected as end index but \(end) actual", + file: (file), + line: line + ) + return returns[everythingState] + } else { XCTFail( "file write call \(fileState) but less than \(expected.count) expected", file: (file), @@ -360,22 +381,6 @@ public final class ChannelTests: XCTestCase { ) return IOResult.wouldBlock(-1 * (everythingState + 1)) } - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - expected[fileState].0, - start, - "in file write \(fileState) (overall \(everythingState)), \(expected[fileState].0) expected as start index but \(start) actual", - file: (file), - line: line - ) - XCTAssertEqual( - expected[fileState].1, - end, - "in file write \(fileState) (overall \(everythingState)), \(expected[fileState].1) expected as end index but \(end) actual", - file: (file), - line: line - ) - return returns[everythingState] } ) if everythingState > 0 { @@ -2211,10 +2216,11 @@ public final class ChannelTests: XCTestCase { try super.init(protocolFamily: protocolFamily, type: .stream, setNonBlocking: true) } override func connect(to address: SocketAddress) throws -> Bool { - guard address.port == 123 else { + if address.port == 123 { + return true + } else { return try super.connect(to: address) } - return true } } class ReadDoesNotHappen: ChannelInboundHandler { @@ -2416,12 +2422,13 @@ public final class ChannelTests: XCTestCase { self.firstReadHappened = true } XCTAssertGreaterThan(pointer.count, 0) - guard self.firstReadHappened else { + if self.firstReadHappened { + // this is a copy of the exact error that'd come out of the real Socket.read + throw IOError.init(errnoCode: ECONNRESET, reason: "read(descriptor:pointer:size:)") + } else { pointer[0] = 0xff return .processed(1) } - // this is a copy of the exact error that'd come out of the real Socket.read - throw IOError.init(errnoCode: ECONNRESET, reason: "read(descriptor:pointer:size:)") } } class VerifyThingsAreRightHandler: ChannelInboundHandler { diff --git a/Tests/NIOPosixTests/CodecTest.swift b/Tests/NIOPosixTests/CodecTest.swift index 097218c0b2..a6ffbe8802 100644 --- a/Tests/NIOPosixTests/CodecTest.swift +++ b/Tests/NIOPosixTests/CodecTest.swift @@ -683,14 +683,15 @@ public final class ByteToMessageDecoderTest: XCTestCase { var state: Int = 1 mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - guard buffer.readSlice(length: self.state) != nil else { + if buffer.readSlice(length: self.state) != nil { + defer { + self.state += 1 + } + context.fireChannelRead(Self.wrapInboundOut(self.state)) + return .continue + } else { return .needMoreData } - defer { - self.state += 1 - } - context.fireChannelRead(Self.wrapInboundOut(self.state)) - return .continue } mutating func decodeLast( @@ -736,19 +737,20 @@ public final class ByteToMessageDecoderTest: XCTestCase { mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { XCTAssertGreaterThan(self.state, 0) - guard let slice = buffer.readSlice(length: self.state) else { - return .needMoreData - } - self.state >>= 1 - for i in 0..>= 1 + for i in 0.. DecodingState { - guard let slice = buffer.readSlice(length: 16) else { + if let slice = buffer.readSlice(length: 16) { + context.fireChannelRead(Self.wrapInboundOut(slice)) + context.channel.close().whenFailure { error in + XCTFail("unexpected error: \(error)") + } + return .continue + } else { return .needMoreData } - context.fireChannelRead(Self.wrapInboundOut(slice)) - context.channel.close().whenFailure { error in - XCTFail("unexpected error: \(error)") - } - return .continue } mutating func decodeLast( @@ -829,14 +832,15 @@ public final class ByteToMessageDecoderTest: XCTestCase { typealias InboundOut = ByteBuffer mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - guard let slice = buffer.readSlice(length: 16) else { + if let slice = buffer.readSlice(length: 16) { + context.fireChannelRead(Self.wrapInboundOut(slice)) + context.pipeline.removeHandler(context: context).whenFailure { error in + XCTFail("unexpected error: \(error)") + } + return .continue + } else { return .needMoreData } - context.fireChannelRead(Self.wrapInboundOut(slice)) - context.pipeline.removeHandler(context: context).whenFailure { error in - XCTFail("unexpected error: \(error)") - } - return .continue } mutating func decodeLast( @@ -875,14 +879,15 @@ public final class ByteToMessageDecoderTest: XCTestCase { typealias InboundOut = ByteBuffer mutating func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - guard let slice = buffer.readSlice(length: 16) else { + if let slice = buffer.readSlice(length: 16) { + context.fireChannelRead(Self.wrapInboundOut(slice)) + context.close().whenFailure { error in + XCTFail("unexpected error: \(error)") + } + return .continue + } else { return .needMoreData } - context.fireChannelRead(Self.wrapInboundOut(slice)) - context.close().whenFailure { error in - XCTFail("unexpected error: \(error)") - } - return .continue } mutating func decodeLast( @@ -1182,11 +1187,12 @@ public final class ByteToMessageDecoderTest: XCTestCase { } func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - guard let string = buffer.readString(length: 1) else { + if let string = buffer.readString(length: 1) { + context.fireChannelRead(Self.wrapInboundOut(string)) + return .continue + } else { return .needMoreData } - context.fireChannelRead(Self.wrapInboundOut(string)) - return .continue } func decodeLast( @@ -1232,19 +1238,20 @@ public final class ByteToMessageDecoderTest: XCTestCase { func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { self.decodeRun += 1 - guard let string = buffer.readString(length: 1) else { + if let string = buffer.readString(length: 1) { + context.fireChannelRead(Self.wrapInboundOut("I: \(self.decodeRun): \(string)")) + XCTAssertNoThrow( + try (context.channel as! EmbeddedChannel).writeOutbound("O: \(self.decodeRun): \(string)") + ) + if self.decodeRun == 1 { + var buffer = context.channel.allocator.buffer(capacity: 1) + buffer.writeStaticString("X") + XCTAssertNoThrow(try (context.channel as! EmbeddedChannel).writeInbound(buffer)) + } + return .continue + } else { return .needMoreData } - context.fireChannelRead(Self.wrapInboundOut("I: \(self.decodeRun): \(string)")) - XCTAssertNoThrow( - try (context.channel as! EmbeddedChannel).writeOutbound("O: \(self.decodeRun): \(string)") - ) - if self.decodeRun == 1 { - var buffer = context.channel.allocator.buffer(capacity: 1) - buffer.writeStaticString("X") - XCTAssertNoThrow(try (context.channel as! EmbeddedChannel).writeInbound(buffer)) - } - return .continue } func decodeLast( @@ -1959,11 +1966,12 @@ private class PairOfBytesDecoder: ByteToMessageDecoder { } func decode(context: ChannelHandlerContext, buffer: inout ByteBuffer) throws -> DecodingState { - guard let slice = buffer.readSlice(length: 2) else { + if let slice = buffer.readSlice(length: 2) { + context.fireChannelRead(Self.wrapInboundOut(slice)) + return .continue + } else { return .needMoreData } - context.fireChannelRead(Self.wrapInboundOut(slice)) - return .continue } func decodeLast(context: ChannelHandlerContext, buffer: inout ByteBuffer, seenEOF: Bool) throws -> DecodingState { @@ -1981,10 +1989,11 @@ public final class MessageToByteHandlerTest: XCTestCase { typealias OutboundIn = Int public func encode(data value: Int, out: inout ByteBuffer) throws { - guard value == 0 else { + if value == 0 { + out.writeInteger(value) + } else { throw HandlerError() } - out.writeInteger(value) } } diff --git a/Tests/NIOPosixTests/DatagramChannelTests.swift b/Tests/NIOPosixTests/DatagramChannelTests.swift index d7c6c3e834..40e0b3dcd0 100644 --- a/Tests/NIOPosixTests/DatagramChannelTests.swift +++ b/Tests/NIOPosixTests/DatagramChannelTests.swift @@ -1206,11 +1206,12 @@ class DatagramChannelTests: XCTestCase { to destinationChannel: Channel, wrappingInAddressedEnvelope shouldWrapInAddressedEnvelope: Bool ) -> EventLoopFuture { - guard shouldWrapInAddressedEnvelope else { + if shouldWrapInAddressedEnvelope { + let envelope = AddressedEnvelope(remoteAddress: destinationChannel.localAddress!, data: data) + return sourceChannel.write(envelope) + } else { return sourceChannel.write(data) } - let envelope = AddressedEnvelope(remoteAddress: destinationChannel.localAddress!, data: data) - return sourceChannel.write(envelope) } func bufferWriteOfHelloWorld( diff --git a/Tests/NIOPosixTests/EchoServerClientTest.swift b/Tests/NIOPosixTests/EchoServerClientTest.swift index cf40bc679e..16b74efc4c 100644 --- a/Tests/NIOPosixTests/EchoServerClientTest.swift +++ b/Tests/NIOPosixTests/EchoServerClientTest.swift @@ -898,13 +898,14 @@ class EchoServerClientTest: XCTestCase { return channel.eventLoop.makeSucceededFuture(()) }.bind(host: host, port: 0).wait() } catch let e as SocketAddressError { - guard case .unknown(host, port: 0) = e else { + if case .unknown(host, port: 0) = e { + // this can happen if the system isn't configured for both IPv4 and IPv6 + continue + } else { // nope, that's a different socket error XCTFail("unexpected SocketAddressError: \(e)") break } - // this can happen if the system isn't configured for both IPv4 and IPv6 - continue } catch { // other unknown error XCTFail("unexpected error: \(error)") diff --git a/Tests/NIOPosixTests/PendingDatagramWritesManagerTests.swift b/Tests/NIOPosixTests/PendingDatagramWritesManagerTests.swift index 82f4a434e5..988b235197 100644 --- a/Tests/NIOPosixTests/PendingDatagramWritesManagerTests.swift +++ b/Tests/NIOPosixTests/PendingDatagramWritesManagerTests.swift @@ -101,54 +101,109 @@ class PendingDatagramWritesManagerTests: XCTestCase { singleState += 1 everythingState += 1 } - guard let expected = expectedSingleWritabilities else { + if let expected = expectedSingleWritabilities { + if expected.count > singleState { + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + expected[singleState].0, + buf.count, + "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].0) bytes expected but \(buf.count) actual", + file: (file), + line: line + ) + XCTAssertEqual( + expected[singleState].1, + addr.map(SocketAddress.init), + "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].1) address expected but \(String(describing: addr.map(SocketAddress.init))) received", + file: (file), + line: line + ) + XCTAssertEqual( + expected[singleState].1.expectedSize, + len, + "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].1.expectedSize) socklen expected but \(len) received", + file: (file), + line: line + ) + + switch returns[everythingState] { + case .success(let r): + return r + case .failure(let e): + throw e + } + } else { + XCTFail( + "single write call \(singleState) but less than \(expected.count) expected", + file: (file), + line: line + ) + return IOResult.wouldBlock(-1 * (everythingState + 1)) + } + } else { XCTFail("single write called on \(buf) but no single writes expected", file: (file), line: line) return IOResult.wouldBlock(-1 * (everythingState + 1)) } - guard expected.count > singleState else { - XCTFail( - "single write call \(singleState) but less than \(expected.count) expected", - file: (file), - line: line - ) - return IOResult.wouldBlock(-1 * (everythingState + 1)) - } - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - expected[singleState].0, - buf.count, - "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].0) bytes expected but \(buf.count) actual", - file: (file), - line: line - ) - XCTAssertEqual( - expected[singleState].1, - addr.map(SocketAddress.init), - "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].1) address expected but \(String(describing: addr.map(SocketAddress.init))) received", - file: (file), - line: line - ) - XCTAssertEqual( - expected[singleState].1.expectedSize, - len, - "in single write \(singleState) (overall \(everythingState)), \(expected[singleState].1.expectedSize) socklen expected but \(len) received", - file: (file), - line: line - ) - - switch returns[everythingState] { - case .success(let r): - return r - case .failure(let e): - throw e - } }, vectorWriteOperation: { ptrs in defer { multiState += 1 everythingState += 1 } - guard let expected = expectedVectorWritabilities else { + if let expected = expectedVectorWritabilities { + if expected.count > multiState { + XCTAssertGreaterThan(returns.count, everythingState) + XCTAssertEqual( + ptrs.map { $0.msg_hdr.msg_iovlen }, + Array(repeating: 1, count: ptrs.count), + "mustn't write more than one iovec element per datagram", + file: (file), + line: line + ) + XCTAssertEqual( + expected[multiState].map { numericCast($0.0) }, + ptrs.map { $0.msg_hdr.msg_iov.pointee.iov_len }, + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.msg_hdr.msg_iov.pointee.iov_len }) actual", + file: (file), + line: line + ) + XCTAssertEqual( + ptrs.map { Int($0.msg_len) }, + Array(repeating: 0, count: ptrs.count), + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.msg_len }) actual", + file: (file), + line: line + ) + XCTAssertEqual( + expected[multiState].map { $0.1 }, + ptrs.map { SocketAddress($0.msg_hdr.msg_name.assumingMemoryBound(to: sockaddr.self)) }, + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState].map { $0.1 }) addresses expected but \(ptrs.map { SocketAddress($0.msg_hdr.msg_name.assumingMemoryBound(to: sockaddr.self)) }) actual", + file: (file), + line: line + ) + XCTAssertEqual( + expected[multiState].map { $0.1.expectedSize }, + ptrs.map { $0.msg_hdr.msg_namelen }, + "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState].map { $0.1.expectedSize }) address lengths expected but \(ptrs.map { $0.msg_hdr.msg_namelen }) actual", + file: (file), + line: line + ) + + switch returns[everythingState] { + case .success(let r): + return r + case .failure(let e): + throw e + } + } else { + XCTFail( + "vector write call \(multiState) but less than \(expected.count) expected", + file: (file), + line: line + ) + return IOResult.wouldBlock(-1 * (everythingState + 1)) + } + } else { XCTFail( "vector write called on \(ptrs) but no vector writes expected", file: (file), @@ -156,57 +211,6 @@ class PendingDatagramWritesManagerTests: XCTestCase { ) return IOResult.wouldBlock(-1 * (everythingState + 1)) } - guard expected.count > multiState else { - XCTFail( - "vector write call \(multiState) but less than \(expected.count) expected", - file: (file), - line: line - ) - return IOResult.wouldBlock(-1 * (everythingState + 1)) - } - XCTAssertGreaterThan(returns.count, everythingState) - XCTAssertEqual( - ptrs.map { $0.msg_hdr.msg_iovlen }, - Array(repeating: 1, count: ptrs.count), - "mustn't write more than one iovec element per datagram", - file: (file), - line: line - ) - XCTAssertEqual( - expected[multiState].map { numericCast($0.0) }, - ptrs.map { $0.msg_hdr.msg_iov.pointee.iov_len }, - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.msg_hdr.msg_iov.pointee.iov_len }) actual", - file: (file), - line: line - ) - XCTAssertEqual( - ptrs.map { Int($0.msg_len) }, - Array(repeating: 0, count: ptrs.count), - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState]) byte counts expected but \(ptrs.map { $0.msg_len }) actual", - file: (file), - line: line - ) - XCTAssertEqual( - expected[multiState].map { $0.1 }, - ptrs.map { SocketAddress($0.msg_hdr.msg_name.assumingMemoryBound(to: sockaddr.self)) }, - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState].map { $0.1 }) addresses expected but \(ptrs.map { SocketAddress($0.msg_hdr.msg_name.assumingMemoryBound(to: sockaddr.self)) }) actual", - file: (file), - line: line - ) - XCTAssertEqual( - expected[multiState].map { $0.1.expectedSize }, - ptrs.map { $0.msg_hdr.msg_namelen }, - "in vector write \(multiState) (overall \(everythingState)), \(expected[multiState].map { $0.1.expectedSize }) address lengths expected but \(ptrs.map { $0.msg_hdr.msg_namelen }) actual", - file: (file), - line: line - ) - - switch returns[everythingState] { - case .success(let r): - return r - case .failure(let e): - throw e - } } ) result = r diff --git a/Tests/NIOPosixTests/SALChannelTests.swift b/Tests/NIOPosixTests/SALChannelTests.swift index 608822fd49..d4d5e45f5c 100644 --- a/Tests/NIOPosixTests/SALChannelTests.swift +++ b/Tests/NIOPosixTests/SALChannelTests.swift @@ -424,18 +424,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel. try self.assertRegister { selectable, eventSet, registration in - guard - case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - else { + if case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + { + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true + } else { return false } - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true } try self.assertReregister { selectable, eventSet in XCTAssertEqual([.reset, .readEOF], eventSet) @@ -497,18 +497,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel from the first accept. try self.assertRegister { selectable, eventSet, registration in - guard - case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - else { + if case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + { + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true + } else { return false } - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true } try self.assertReregister { selectable, eventSet in XCTAssertEqual([.reset, .readEOF], eventSet) @@ -808,18 +808,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel. try self.assertRegister { selectable, eventSet, registration in - guard - case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - else { + if case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + { + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true + } else { return false } - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true } try self.assertReregister { selectable, event in XCTAssertEqual([.reset, .readEOF], event) @@ -888,18 +888,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel. try self.assertRegister { selectable, eventSet, registration in - guard - case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - else { + if case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + { + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true + } else { return false } - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true } try self.assertReregister { selectable, event in XCTAssertEqual([.reset, .readEOF], event) @@ -976,18 +976,18 @@ final class SALChannelTest: XCTestCase, SALTest { // Then we register the inbound channel. try self.assertRegister { selectable, eventSet, registration in - guard - case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - else { + if case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + { + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true + } else { return false } - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true } try self.assertReregister { selectable, event in XCTAssertEqual([.reset, .readEOF], event) diff --git a/Tests/NIOPosixTests/SocketChannelTest.swift b/Tests/NIOPosixTests/SocketChannelTest.swift index 5391f73cf0..80133abbdd 100644 --- a/Tests/NIOPosixTests/SocketChannelTest.swift +++ b/Tests/NIOPosixTests/SocketChannelTest.swift @@ -757,14 +757,15 @@ public final class SocketChannelTest: XCTestCase { let shouldAcceptsFail = ManagedAtomic(true) override func accept(setNonBlocking: Bool = false) throws -> Socket? { XCTAssertTrue(setNonBlocking) - guard self.shouldAcceptsFail.load(ordering: .relaxed) else { + if self.shouldAcceptsFail.load(ordering: .relaxed) { + throw NIOFcntlFailedError() + } else { return try Socket( protocolFamily: .inet, type: .stream, setNonBlocking: false ) } - throw NIOFcntlFailedError() } } diff --git a/Tests/NIOPosixTests/SyscallAbstractionLayer.swift b/Tests/NIOPosixTests/SyscallAbstractionLayer.swift index f2aa8ad14d..ab5b075110 100644 --- a/Tests/NIOPosixTests/SyscallAbstractionLayer.swift +++ b/Tests/NIOPosixTests/SyscallAbstractionLayer.swift @@ -83,35 +83,38 @@ final class LockedBox { } func waitForEmptyAndSet(_ value: T) throws { - guard self.condition.lock(whenValue: 0, timeoutSeconds: SAL.defaultTimeout) else { + if self.condition.lock(whenValue: 0, timeoutSeconds: SAL.defaultTimeout) { + defer { + self.condition.unlock(withValue: 1) + } + self._value = value + } else { throw TimeoutError(self.description) } - defer { - self.condition.unlock(withValue: 1) - } - self._value = value } func takeValue() throws -> T { - guard self.condition.lock(whenValue: 1, timeoutSeconds: SAL.defaultTimeout) else { + if self.condition.lock(whenValue: 1, timeoutSeconds: SAL.defaultTimeout) { + defer { + self.condition.unlock(withValue: 0) + } + let value = self._value! + self._value = nil + return value + } else { throw TimeoutError(self.description) } - defer { - self.condition.unlock(withValue: 0) - } - let value = self._value! - self._value = nil - return value } func waitForValue() throws -> T { - guard self.condition.lock(whenValue: 1, timeoutSeconds: SAL.defaultTimeout) else { + if self.condition.lock(whenValue: 1, timeoutSeconds: SAL.defaultTimeout) { + defer { + self.condition.unlock(withValue: 1) + } + return self._value! + } else { throw TimeoutError(self.description) } - defer { - self.condition.unlock(withValue: 1) - } - return self._value! } } @@ -183,10 +186,11 @@ private protocol UserKernelInterface { extension UserKernelInterface { fileprivate func waitForKernelReturn() throws -> KernelToUser { let value = try self.kernelToUser.takeValue() - guard case .error(let error) = value else { + if case .error(let error) = value { + throw error + } else { return value } - throw error } } @@ -219,19 +223,21 @@ internal class HookedSelector: NIOPosix.Selector, UserKernelInt ) ) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } override func reregister(selectable: S, interested: SelectorEventSet) throws { try self.userToKernel.waitForEmptyAndSet(.reregister(selectable, interested)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } override func whenReady( @@ -241,23 +247,25 @@ internal class HookedSelector: NIOPosix.Selector, UserKernelInt ) throws { try self.userToKernel.waitForEmptyAndSet(.whenReady(strategy)) let ret = try self.waitForKernelReturn() - guard case .returnSelectorEvent(let event) = ret else { + if case .returnSelectorEvent(let event) = ret { + loopStart() + if let event = event { + try body(event) + } + return + } else { throw UnexpectedKernelReturn(ret) } - loopStart() - if let event = event { - try body(event) - } - return } override func deregister(selectable: S) throws { try self.userToKernel.waitForEmptyAndSet(.deregister(selectable)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } override func wakeup() throws { @@ -284,48 +292,53 @@ class HookedServerSocket: ServerSocket, UserKernelInterface { try self.withUnsafeHandle { fd in try self.userToKernel.waitForEmptyAndSet(.disableSIGPIPE(fd)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } } override func localAddress() throws -> SocketAddress { try self.userToKernel.waitForEmptyAndSet(.localAddress) let ret = try self.waitForKernelReturn() - guard case .returnSocketAddress(let address) = ret else { + if case .returnSocketAddress(let address) = ret { + return address + } else { throw UnexpectedKernelReturn(ret) } - return address } override func remoteAddress() throws -> SocketAddress { try self.userToKernel.waitForEmptyAndSet(.remoteAddress) let ret = try self.waitForKernelReturn() - guard case .returnSocketAddress(let address) = ret else { + if case .returnSocketAddress(let address) = ret { + return address + } else { throw UnexpectedKernelReturn(ret) } - return address } override func bind(to address: SocketAddress) throws { try self.userToKernel.waitForEmptyAndSet(.bind(address)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } override func listen(backlog: Int32 = 128) throws { try self.withUnsafeHandle { fd in try self.userToKernel.waitForEmptyAndSet(.listen(fd, backlog)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } } @@ -349,10 +362,11 @@ class HookedServerSocket: ServerSocket, UserKernelInterface { try self.userToKernel.waitForEmptyAndSet(.close(fd)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } } @@ -374,49 +388,54 @@ class HookedSocket: Socket, UserKernelInterface { try self.withUnsafeHandle { fd in try self.userToKernel.waitForEmptyAndSet(.disableSIGPIPE(fd)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } } override func localAddress() throws -> SocketAddress { try self.userToKernel.waitForEmptyAndSet(.localAddress) let ret = try self.waitForKernelReturn() - guard case .returnSocketAddress(let address) = ret else { + if case .returnSocketAddress(let address) = ret { + return address + } else { throw UnexpectedKernelReturn(ret) } - return address } override func remoteAddress() throws -> SocketAddress { try self.userToKernel.waitForEmptyAndSet(.remoteAddress) let ret = try self.waitForKernelReturn() - guard case .returnSocketAddress(let address) = ret else { + if case .returnSocketAddress(let address) = ret { + return address + } else { throw UnexpectedKernelReturn(ret) } - return address } override func connect(to address: SocketAddress) throws -> Bool { try self.userToKernel.waitForEmptyAndSet(.connect(address)) let ret = try self.waitForKernelReturn() - guard case .returnBool(let success) = ret else { + if case .returnBool(let success) = ret { + return success + } else { throw UnexpectedKernelReturn(ret) } - return success } override func read(pointer: UnsafeMutableRawBufferPointer) throws -> IOResult { try self.userToKernel.waitForEmptyAndSet(.read(pointer.count)) let ret = try self.waitForKernelReturn() - guard case .returnBytes(let buffer) = ret else { + if case .returnBytes(let buffer) = ret { + assert(buffer.readableBytes <= pointer.count) + pointer.copyBytes(from: buffer.readableBytesView) + return .processed(buffer.readableBytes) + } else { throw UnexpectedKernelReturn(ret) } - assert(buffer.readableBytes <= pointer.count) - pointer.copyBytes(from: buffer.readableBytesView) - return .processed(buffer.readableBytes) } override func write(pointer: UnsafeRawBufferPointer) throws -> IOResult { @@ -425,10 +444,11 @@ class HookedSocket: Socket, UserKernelInterface { buffer.writeBytes(pointer) try self.userToKernel.waitForEmptyAndSet(.write(fd, buffer)) let ret = try self.waitForKernelReturn() - guard case .returnIOResultInt(let result) = ret else { + if case .returnIOResultInt(let result) = ret { + return result + } else { throw UnexpectedKernelReturn(ret) } - return result } } @@ -447,10 +467,11 @@ class HookedSocket: Socket, UserKernelInterface { try self.userToKernel.waitForEmptyAndSet(.writev(fd, buffers)) let ret = try self.waitForKernelReturn() - guard case .returnIOResultInt(let result) = ret else { + if case .returnIOResultInt(let result) = ret { + return result + } else { throw UnexpectedKernelReturn(ret) } - return result } } @@ -459,37 +480,41 @@ class HookedSocket: Socket, UserKernelInterface { try self.userToKernel.waitForEmptyAndSet(.close(fd)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } override func getOption(level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option) throws -> T { try self.userToKernel.waitForEmptyAndSet(.getOption(level, name)) let ret = try self.waitForKernelReturn() - guard case .returnAny(let any) = ret else { + if case .returnAny(let any) = ret { + return any as! T + } else { throw UnexpectedKernelReturn(ret) } - return any as! T } override func setOption(level: NIOBSDSocket.OptionLevel, name: NIOBSDSocket.Option, value: T) throws { try self.userToKernel.waitForEmptyAndSet(.setOption(level, name, value)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } override func bind(to address: SocketAddress) throws { try self.userToKernel.waitForEmptyAndSet(.bind(address)) let ret = try self.waitForKernelReturn() - guard case .returnVoid = ret else { + if case .returnVoid = ret { + return + } else { throw UnexpectedKernelReturn(ret) } - return } } @@ -501,11 +526,12 @@ extension HookedSelector { matcher: (UserToKernel) throws -> Bool ) throws { let syscall = try self.userToKernel.takeValue() - guard try matcher(syscall) else { + if try matcher(syscall) { + try self.kernelToUser.waitForEmptyAndSet(result) + } else { XCTFail("unexpected syscall \(syscall)", file: (file), line: line) throw UnexpectedSyscall(syscall) } - try self.kernelToUser.waitForEmptyAndSet(result) } /// This function will wait for an event loop wakeup until it unblocks. If the event loop @@ -516,10 +542,11 @@ extension HookedSelector { SAL.printIfDebug("\(#function)") try self.wakeups.takeValue() try self.assertSyscallAndReturn(.returnSelectorEvent(nil), file: (file), line: line) { syscall in - guard case .whenReady(.block) = syscall else { + if case .whenReady(.block) = syscall { + return true + } else { return false } - return true } } @@ -719,18 +746,18 @@ extension SALTest { try self.assertLocalAddress(address: localAddress) try self.assertRemoteAddress(address: remoteAddress) try self.assertRegister { selectable, eventSet, registration in - guard - case (.socketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - else { + if case (.socketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + { + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(remoteAddress, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true + } else { return false } - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(remoteAddress, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true } try self.assertReregister { selectable, eventSet in XCTAssertEqual([.reset, .readEOF], eventSet) @@ -761,18 +788,18 @@ extension SALTest { try self.assertLocalAddress(address: localAddress) try self.assertListen(expectedFD: .max, expectedBacklog: 128) try self.assertRegister { selectable, eventSet, registration in - guard - case (.serverSocketChannel(let channel), let registrationEventSet) = - (registration.channel, registration.interested) - else { + if case (.serverSocketChannel(let channel), let registrationEventSet) = + (registration.channel, registration.interested) + { + + XCTAssertEqual(localAddress, channel.localAddress) + XCTAssertEqual(nil, channel.remoteAddress) + XCTAssertEqual(eventSet, registrationEventSet) + XCTAssertEqual(.reset, eventSet) + return true + } else { return false } - - XCTAssertEqual(localAddress, channel.localAddress) - XCTAssertEqual(nil, channel.remoteAddress) - XCTAssertEqual(eventSet, registrationEventSet) - XCTAssertEqual(.reset, eventSet) - return true } try self.assertReregister { selectable, eventSet in XCTAssertEqual([.reset, .readEOF], eventSet) @@ -842,10 +869,11 @@ extension SALTest { file: (file), line: line ) { syscall in - guard case .whenReady = syscall else { + if case .whenReady = syscall { + return true + } else { return false } - return true } } @@ -868,10 +896,11 @@ extension SALTest { ret = .error(error) } try self.selector.assertSyscallAndReturn(ret, file: (file), line: line) { syscall in - guard case .disableSIGPIPE(expectedFD) = syscall else { + if case .disableSIGPIPE(expectedFD) = syscall { + return true + } else { return false } - return true } } @@ -884,10 +913,11 @@ extension SALTest { file: (file), line: line ) { syscall in - guard case .localAddress = syscall else { + if case .localAddress = syscall { + return true + } else { return false } - return true } } @@ -898,10 +928,11 @@ extension SALTest { file: (file), line: line ) { syscall in - guard case .remoteAddress = syscall else { + if case .remoteAddress = syscall { + return true + } else { return false } - return true } } @@ -914,31 +945,34 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnBool(result), file: (file), line: line) { syscall in - guard case .connect(let address) = syscall else { + if case .connect(let address) = syscall { + return address == expectedAddress + } else { return false } - return address == expectedAddress } } func assertBind(expectedAddress: SocketAddress, file: StaticString = #filePath, line: UInt = #line) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - guard case .bind(let address) = syscall else { + if case .bind(let address) = syscall { + return address == expectedAddress + } else { return false } - return address == expectedAddress } } func assertClose(expectedFD: CInt, file: StaticString = #filePath, line: UInt = #line) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - guard case .close(let fd) = syscall else { + if case .close(let fd) = syscall { + XCTAssertEqual(expectedFD, fd, file: (file), line: line) + return true + } else { return false } - XCTAssertEqual(expectedFD, fd, file: (file), line: line) - return true } } @@ -951,10 +985,11 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnAny(value), file: (file), line: line) { syscall in - guard case .getOption(expectedLevel, expectedOption) = syscall else { + if case .getOption(expectedLevel, expectedOption) = syscall { + return true + } else { return false } - return true } } @@ -967,10 +1002,11 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - guard case .setOption(expectedLevel, expectedOption, let value) = syscall else { + if case .setOption(expectedLevel, expectedOption, let value) = syscall { + return valueMatcher(value) + } else { return false } - return valueMatcher(value) } } @@ -981,10 +1017,11 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - guard case .register(let selectable, let eventSet, let registration) = syscall else { + if case .register(let selectable, let eventSet, let registration) = syscall { + return try matcher(selectable, eventSet, registration) + } else { return false } - return try matcher(selectable, eventSet, registration) } } @@ -995,10 +1032,11 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - guard case .reregister(let selectable, let eventSet) = syscall else { + if case .reregister(let selectable, let eventSet) = syscall { + return try matcher(selectable, eventSet) + } else { return false } - return try matcher(selectable, eventSet) } } @@ -1009,10 +1047,11 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnVoid, file: (file), line: line) { syscall in - guard case .deregister(let selectable) = syscall else { + if case .deregister(let selectable) = syscall { + return try matcher(selectable) + } else { return false } - return try matcher(selectable) } } @@ -1025,10 +1064,11 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnIOResultInt(`return`), file: (file), line: line) { syscall in - guard case .write(let actualFD, let actualBytes) = syscall else { + if case .write(let actualFD, let actualBytes) = syscall { + return expectedFD == actualFD && expectedBytes == actualBytes + } else { return false } - return expectedFD == actualFD && expectedBytes == actualBytes } } @@ -1041,10 +1081,11 @@ extension SALTest { ) throws { SAL.printIfDebug("\(#function)") try self.selector.assertSyscallAndReturn(.returnIOResultInt(`return`), file: (file), line: line) { syscall in - guard case .writev(let actualFD, let actualBytes) = syscall else { + if case .writev(let actualFD, let actualBytes) = syscall { + return expectedFD == actualFD && expectedBytes == actualBytes + } else { return false } - return expectedFD == actualFD && expectedBytes == actualBytes } } @@ -1061,11 +1102,12 @@ extension SALTest { file: (file), line: line ) { syscall in - guard case .read(let amount) = syscall else { + if case .read(let amount) = syscall { + XCTAssertEqual(expectedBufferSpace, amount, file: (file), line: line) + return true + } else { return false } - XCTAssertEqual(expectedBufferSpace, amount, file: (file), line: line) - return true } } @@ -1081,12 +1123,13 @@ extension SALTest { file: (file), line: line ) { syscall in - guard case .listen(let fd, let backlog) = syscall else { + if case .listen(let fd, let backlog) = syscall { + XCTAssertEqual(fd, expectedFD, file: (file), line: line) + XCTAssertEqual(backlog, expectedBacklog, file: (file), line: line) + return true + } else { return false } - XCTAssertEqual(fd, expectedFD, file: (file), line: line) - XCTAssertEqual(backlog, expectedBacklog, file: (file), line: line) - return true } } @@ -1103,12 +1146,13 @@ extension SALTest { file: (file), line: line ) { syscall in - guard case .accept(let fd, let nonBlocking) = syscall else { + if case .accept(let fd, let nonBlocking) = syscall { + XCTAssertEqual(fd, expectedFD, file: (file), line: line) + XCTAssertEqual(nonBlocking, expectedNonBlocking, file: (file), line: line) + return true + } else { return false } - XCTAssertEqual(fd, expectedFD, file: (file), line: line) - XCTAssertEqual(nonBlocking, expectedNonBlocking, file: (file), line: line) - return true } } @@ -1125,12 +1169,13 @@ extension SALTest { file: (file), line: line ) { syscall in - guard case .accept(let fd, let nonBlocking) = syscall else { + if case .accept(let fd, let nonBlocking) = syscall { + XCTAssertEqual(fd, expectedFD, file: (file), line: line) + XCTAssertEqual(nonBlocking, expectedNonBlocking, file: (file), line: line) + return true + } else { return false } - XCTAssertEqual(fd, expectedFD, file: (file), line: line) - XCTAssertEqual(nonBlocking, expectedNonBlocking, file: (file), line: line) - return true } } diff --git a/Tests/NIOPosixTests/SystemCallWrapperHelpers.swift b/Tests/NIOPosixTests/SystemCallWrapperHelpers.swift index f2ccfc57f1..d2041f9286 100644 --- a/Tests/NIOPosixTests/SystemCallWrapperHelpers.swift +++ b/Tests/NIOPosixTests/SystemCallWrapperHelpers.swift @@ -88,21 +88,22 @@ func runSystemCallWrapperPerformanceTest( for _ in 0..( return try body() } catch { XCTFail("\(message.map { $0 + ": " } ?? "")unexpected error \(error) thrown", file: (file), line: line) - guard let defaultValue = defaultValue else { + if let defaultValue = defaultValue { + return defaultValue + } else { throw error } - return defaultValue } } @@ -840,7 +842,15 @@ func forEachCrossConnectedStreamChannelPair( extension EventLoopFuture { var isFulfilled: Bool { - guard self.eventLoop.inEventLoop else { + if self.eventLoop.inEventLoop { + // Easy, we're on the EventLoop. Let's just use our knowledge that we run completed future callbacks + // immediately. + var fulfilled = false + self.whenComplete { _ in + fulfilled = true + } + return fulfilled + } else { let lock = NIOLock() let group = DispatchGroup() var fulfilled = false // protected by lock @@ -856,12 +866,5 @@ extension EventLoopFuture { group.wait() // this is very nasty but this is for tests only, so... return lock.withLock { fulfilled } } - // Easy, we're on the EventLoop. Let's just use our knowledge that we run completed future callbacks - // immediately. - var fulfilled = false - self.whenComplete { _ in - fulfilled = true - } - return fulfilled } } diff --git a/Tests/NIOTestUtilsTests/NIOHTTP1TestServerTest.swift b/Tests/NIOTestUtilsTests/NIOHTTP1TestServerTest.swift index 734882e89c..293fb624bc 100644 --- a/Tests/NIOTestUtilsTests/NIOHTTP1TestServerTest.swift +++ b/Tests/NIOTestUtilsTests/NIOHTTP1TestServerTest.swift @@ -547,9 +547,10 @@ func assertNoThrowWithValue( return try body() } catch { XCTFail("\(message.map { $0 + ": " } ?? "")unexpected error \(error) thrown", file: (file), line: line) - guard let defaultValue = defaultValue else { + if let defaultValue = defaultValue { + return defaultValue + } else { throw error } - return defaultValue } } diff --git a/Tests/NIOWebSocketTests/WebSocketFrameDecoderTest.swift b/Tests/NIOWebSocketTests/WebSocketFrameDecoderTest.swift index 8774aabe17..8a0a7f1944 100644 --- a/Tests/NIOWebSocketTests/WebSocketFrameDecoderTest.swift +++ b/Tests/NIOWebSocketTests/WebSocketFrameDecoderTest.swift @@ -115,10 +115,11 @@ public final class WebSocketFrameDecoderTest: XCTestCase { // an encoder because we want to fail gracefully if a frame is written. let f = self.decoderChannel.pipeline.context(handlerType: ByteToMessageHandler.self) .flatMapThrowing { - guard let handler = $0.handler as? RemovableChannelHandler else { + if let handler = $0.handler as? RemovableChannelHandler { + return handler + } else { throw ChannelError.unremovableHandler } - return handler }.flatMap { self.decoderChannel.pipeline.removeHandler($0) }