stream channel initialiser: allow closed & failed (#183)

Motivation:

Previously, if we failed in the stream channel initialiser and the
stream channel was closed before finishing the stream channel
initialiser, we would crash on an assertion because `closedWhileOpen`
correctly asserted that it has nothing to do and shouldn't be called.

Modification:

Apart from `.idle` also stop calling `closedWhileOpen` in `.closed` and
`.localActive` because the network actually doesn't think this channel
is open.

Result:

- Fewer crashes
- fixes rdar://59342916

Author: weissi

Sources/NIOHTTP2/HTTP2StreamChannel.swift

@@ -169,10 +169,13 @@ final class HTTP2StreamChannel: Channel, ChannelCore {
         }
 
         f.whenFailure { (error: Error) in
-            if self.state != .idle {
-                self.closedWhileOpen()
-            } else {
+            switch self.state {
+            case .idle, .localActive, .closed:
+                // The stream isn't open on the network, nothing to close.
                 self.errorEncountered(error: error)
+            case .remoteActive, .active, .closing, .closingNeverActivated:
+                // In all of these states the stream is still on the network and we may need to take action.
+                self.closedWhileOpen()
             }
 
             promise?.fail(error)

Tests/NIOHTTP2Tests/HTTP2StreamMultiplexerTests+XCTest.swift

@@ -70,6 +70,7 @@ extension HTTP2StreamMultiplexerTests {
                 ("testMultiplexerModifiesStreamChannelWritabilityBasedOnFixedSizeTokens", testMultiplexerModifiesStreamChannelWritabilityBasedOnFixedSizeTokens),
                 ("testMultiplexerModifiesStreamChannelWritabilityBasedOnParentChannelWritability", testMultiplexerModifiesStreamChannelWritabilityBasedOnParentChannelWritability),
                 ("testMultiplexerModifiesStreamChannelWritabilityBasedOnFixedSizeTokensAndChannelWritability", testMultiplexerModifiesStreamChannelWritabilityBasedOnFixedSizeTokensAndChannelWritability),
+                ("testStreamChannelToleratesFailingInitialier", testStreamChannelToleratesFailingInitialier),
            ]
    }
 }

Tests/NIOHTTP2Tests/HTTP2StreamMultiplexerTests.swift

@@ -1773,4 +1773,29 @@ final class HTTP2StreamMultiplexerTests: XCTestCase {
         self.channel.pipeline.fireChannelWritabilityChanged()
         XCTAssertTrue(childChannel.isWritable)
     }
+
+    func testStreamChannelToleratesFailingInitialier() {
+        struct DummyError: Error {}
+        let multiplexer = HTTP2StreamMultiplexer(mode: .client,
+                                                 channel: self.channel,
+                                                 outboundBufferSizeHighWatermark: 100,
+                                                 outboundBufferSizeLowWatermark: 50) { (_, _) in
+            XCTFail("Must not be called")
+            return self.channel.eventLoop.makeFailedFuture(MyError())
+        }
+        XCTAssertNoThrow(try self.channel.pipeline.addHandler(multiplexer).wait())
+
+        // We need to activate the underlying channel here.
+        XCTAssertNoThrow(try self.channel.connect(to: SocketAddress(ipAddress: "1.2.3.4", port: 5)).wait())
+
+        // Now we want to create a new child stream.
+        let childChannelPromise = self.channel.eventLoop.makePromise(of: Channel.self)
+        multiplexer.createStreamChannel(promise: childChannelPromise) { childChannel, _ in
+            childChannel.close().flatMap {
+                childChannel.eventLoop.makeFailedFuture(DummyError())
+            }
+        }
+        self.channel.embeddedEventLoop.run()
+    }
+
 }