Avoid a double-idle in the idle handler.

Sources/GRPC/GRPCIdleHandler.swift

@@ -97,6 +97,10 @@ internal class GRPCIdleHandler: ChannelInboundHandler {
     context.fireChannelActive()
   }
 
+  func handlerRemoved(context: ChannelHandlerContext) {
+    self.scheduledIdle?.cancel()
+  }
+
   func channelInactive(context: ChannelHandlerContext) {
     self.scheduledIdle?.cancel()
     self.scheduledIdle = nil
@@ -166,18 +170,26 @@ internal class GRPCIdleHandler: ChannelInboundHandler {
   }
 
   private func idle(context: ChannelHandlerContext) {
+    // Don't idle if there are active streams.
     guard self.activeStreams == 0 else {
       return
     }
 
-    self.state = .closed
-    switch self.mode {
-    case .client(let manager):
-      manager.idle()
-    case .server:
+    switch self.state {
+    case .notReady, .ready:
+      self.state = .closed
+      switch self.mode {
+      case .client(let manager):
+        manager.idle()
+      case .server:
+        ()
+      }
+      context.close(mode: .all, promise: nil)
+
+    // We need to guard against double closure here. We may go idle as a result of receiving a
+    // GOAWAY frame or because our scheduled idle timeout fired. 
+    case .closed:
       ()
     }
-
-    context.close(mode: .all, promise: nil)
   }
 }

Tests/GRPCTests/ConnectionManagerTests.swift

@@ -687,6 +687,61 @@ extension ConnectionManagerTests {
     self.loop.run()
     XCTAssertThrowsError(try channel.wait())
   }
+
+  func testDoubleIdle() throws {
+    class CloseDroppingHandler: ChannelOutboundHandler {
+      typealias OutboundIn = Any
+      func close(context: ChannelHandlerContext, mode: CloseMode, promise: EventLoopPromise<Void>?) {
+        promise?.fail(GRPCStatus(code: .unavailable, message: "Purposefully dropping channel close"))
+      }
+    }
+
+    let channelPromise = self.loop.makePromise(of: Channel.self)
+    let manager = ConnectionManager.testingOnly(configuration: self.defaultConfiguration, logger: self.logger) {
+      return channelPromise.futureResult
+    }
+
+    // Start the connection.
+    let readyChannel: EventLoopFuture<Channel> = self.waitForStateChange(from: .idle, to: .connecting) {
+      let readyChannel = manager.getChannel()
+      self.loop.run()
+      return readyChannel
+    }
+
+    // Setup the real channel and activate it.
+    let channel = EmbeddedChannel(loop: self.loop)
+    XCTAssertNoThrow(try channel.pipeline.addHandlers([
+      CloseDroppingHandler(),
+      GRPCIdleHandler(mode: .client(manager))
+    ]).wait())
+    channelPromise.succeed(channel)
+    self.loop.run()
+    XCTAssertNoThrow(try channel.connect(to: SocketAddress(unixDomainSocketPath: "/ignored")).wait())
+
+    // Write a SETTINGS frame on the root stream.
+    try self.waitForStateChange(from: .connecting, to: .ready) {
+      let frame = HTTP2Frame(streamID: .rootStream, payload: .settings(.settings([])))
+      XCTAssertNoThrow(try channel.writeInbound(frame))
+    }
+
+    // The channel should now be ready.
+    XCTAssertNoThrow(try readyChannel.wait())
+
+    // Send a GO_AWAY; the details don't matter. This will cause the connection to go idle and the
+    // channel to close.
+    try self.waitForStateChange(from: .ready, to: .idle) {
+      let goAway = HTTP2Frame(
+        streamID: .rootStream,
+        payload: .goAway(lastStreamID: 1, errorCode: .noError, opaqueData: nil)
+      )
+      XCTAssertNoThrow(try channel.writeInbound(goAway))
+    }
+
+    // We dropped the close; now wait for the scheduled idle to fire.
+    //
+    // Previously doing this this would fail a precondition.
+    self.loop.advanceTime(by: .minutes(5))
+  }
 }
 
 internal struct Change: Hashable, CustomStringConvertible {

Tests/GRPCTests/XCTestManifests.swift

@@ -154,6 +154,7 @@ extension ConnectionManagerTests {
         ("testConnectOnSecondAttempt", testConnectOnSecondAttempt),
         ("testDoomedOptimisticChannelFromConnecting", testDoomedOptimisticChannelFromConnecting),
         ("testDoomedOptimisticChannelFromIdle", testDoomedOptimisticChannelFromIdle),
+        ("testDoubleIdle", testDoubleIdle),
         ("testGoAwayWhenReady", testGoAwayWhenReady),
         ("testIdleShutdown", testIdleShutdown),
         ("testIdleTimeoutWhenThereAreActiveStreams", testIdleTimeoutWhenThereAreActiveStreams),