Browse Source

Notify of quiescing when there are no open streams (#1819)

Gustavo Cairo 1 year ago
parent
commit
61b7262e53

+ 4 - 2
Sources/GRPC/GRPCIdleHandlerStateMachine.swift

@@ -414,12 +414,12 @@ struct GRPCIdleHandlerStateMachine {
 
     switch self.state {
     case let .operating(state):
+      operations.notifyConnectionManager(about: .quiescing)
       if state.hasOpenStreams {
         // There are open streams: send a GOAWAY frame and wait for the stream count to reach zero.
         //
         // It's okay if we haven't seen a SETTINGS frame at this point; we've initiated the shutdown
         // so making a connection is ready isn't necessary.
-        operations.notifyConnectionManager(about: .quiescing)
 
         // TODO: we should ratchet down the last initiated stream after 1-RTT.
         //
@@ -475,8 +475,8 @@ struct GRPCIdleHandlerStateMachine {
       // A SETTINGS frame MUST follow the connection preface. (RFC 7540 § 3.5)
       assert(state.hasSeenSettings)
 
+      operations.notifyConnectionManager(about: .quiescing)
       if state.hasOpenStreams {
-        operations.notifyConnectionManager(about: .quiescing)
         switch state.role {
         case .client:
           // The server sent us a GOAWAY we'll just stop opening new streams and will send a GOAWAY
@@ -500,7 +500,9 @@ struct GRPCIdleHandlerStateMachine {
     case let .waitingToIdle(state):
       // There can't be any open streams, but we have a few loose ends to clear up: we need to
       // cancel the idle timeout, send a GOAWAY frame and then close.
+      // We should also notify the connection manager that quiescing is happening.
       self.state = .closing(.init(fromWaitingToIdle: state))
+      operations.notifyConnectionManager(about: .quiescing)
       operations.cancelIdleTask(state.idleTask)
       operations.sendGoAwayFrame(lastPeerInitiatedStreamID: state.lastPeerInitiatedStreamID)
       operations.closeChannel()

+ 6 - 1
Tests/GRPCTests/GRPCIdleHandlerStateMachineTests.swift

@@ -194,6 +194,7 @@ class GRPCIdleHandlerStateMachineTests: GRPCTestCase {
     op3.assertGoAway(streamID: .rootStream)
     op3.assertShouldClose()
     op3.assertCancelIdleTimeout()
+    op3.assertConnectionManager(.quiescing)
 
     // Close; we were going to go idle anyway.
     let op4 = stateMachine.channelInactive()
@@ -207,6 +208,7 @@ class GRPCIdleHandlerStateMachineTests: GRPCTestCase {
     let op1 = stateMachine.initiateGracefulShutdown()
     op1.assertGoAway(streamID: .rootStream)
     op1.assertShouldClose()
+    op1.assertConnectionManager(.quiescing)
 
     // Closed.
     let op2 = stateMachine.channelInactive()
@@ -223,6 +225,7 @@ class GRPCIdleHandlerStateMachineTests: GRPCTestCase {
     // Initiate shutdown.
     let op2 = stateMachine.initiateGracefulShutdown()
     op2.assertShouldNotClose()
+    op2.assertConnectionManager(.quiescing)
 
     // Receive a GOAWAY; no change.
     let op3 = stateMachine.receiveGoAway()
@@ -467,8 +470,9 @@ class GRPCIdleHandlerStateMachineTests: GRPCTestCase {
     let op5 = stateMachine.receiveGoAway()
     // We're the client, there are no server initiated streams, so GOAWAY with root stream.
     op5.assertGoAway(streamID: 0)
-    // No open streams, so we can close now.
+    // No open streams, so we can close now. Also assert the connection manager got a quiescing event.
     op5.assertShouldClose()
+    op5.assertConnectionManager(.quiescing)
 
     // Closed.
     let op6 = stateMachine.channelInactive()
@@ -495,6 +499,7 @@ class GRPCIdleHandlerStateMachineTests: GRPCTestCase {
     let op4 = stateMachine.receiveGoAway()
     op4.assertGoAway(streamID: .maxID)
     op4.assertShouldPingAfterGoAway()
+    op4.assertConnectionManager(.quiescing)
 
     // Create another stream. This is fine, the client hasn't ack'd the ping yet.
     let op5 = stateMachine.streamCreated(withID: 7)