review comments

Author: rnro

Sources/NIOCore/SingleStepByteToMessageDecoder.swift

@@ -233,8 +233,9 @@ public final class NIOSingleStepByteToMessageProcessor<Decoder: NIOSingleStepByt
         }
 
         self._buffer = nil  // To avoid CoW
+        defer { self._buffer = buffer }
+
         let result = try body(&buffer)
-        self._buffer = buffer
         return result
     }
 
@@ -272,7 +273,6 @@ public final class NIOSingleStepByteToMessageProcessor<Decoder: NIOSingleStepByt
             throw ByteToMessageDecoderError.PayloadTooLargeError()
         }
 
-        // force unwrapping is safe because nil case is handled already and asserted above
         if let readerIndex = self._buffer?.readerIndex, readerIndex > 0, self.decoder.shouldReclaimBytes(buffer: self._buffer!) {
             self._buffer!.discardReadBytes()
         }

Tests/NIOCoreTests/SingleStepByteToMessageDecoderTest.swift

@@ -564,6 +564,7 @@ public final class NIOSingleStepByteToMessageDecoderTest: XCTestCase {
             struct DecodeLastError: Error {}
 
             func decodeLast(buffer: inout NIOCore.ByteBuffer, seenEOF: Bool) throws -> NIOCore.ByteBuffer? {
+                buffer = ByteBuffer() // to allow the decode loop to exit
                 throw DecodeLastError()
             }
 
@@ -574,10 +575,16 @@ public final class NIOSingleStepByteToMessageDecoderTest: XCTestCase {
 
         let decoder = ThrowingOnLastDecoder()
         let b2mp = NIOSingleStepByteToMessageProcessor(decoder)
+        var errorObserved = false
         XCTAssertNoThrow(try b2mp.process(buffer: ByteBuffer(string: "1\n\n2\n3\n")) { line in
-            XCTAssertThrowsError(try b2mp.finishProcessing(seenEOF: true) { _ in }) { error in
-                XCTAssertNotNil(error as? ThrowingOnLastDecoder.DecodeLastError)
+            // We will throw an error to exit the decoding within the nested process call prematurely.
+            // Unless this is carefully handled we can be left in an inconsistent state which the outer call will encounter
+            do {
+                try b2mp.finishProcessing(seenEOF: true) { _ in }
+            } catch _ as ThrowingOnLastDecoder.DecodeLastError {
+                errorObserved = true
             }
         })
+        XCTAssertTrue(errorObserved)
     }
 }