Skip to content

Commit 4033c04

Browse files
glbrnttbimawa
authored andcommitted
Ignore state machine inputs in unexpected states (grpc#1374)
Motivation: When we ratchet down the last stream ID in a GOAWAY frame, we should only do it if we're quiescing. If we aren't quiescing then we should just ignore the request to do it. Modifications: - Ignore requests to ratchet down the last stream ID in a GOAWAY if we're not quiescing. Result: Fewer traps.
1 parent e087b7a commit 4033c04

File tree

3 files changed

+26
-5
lines changed

3 files changed

+26
-5
lines changed

Sources/GRPC/GRPCIdleHandlerStateMachine.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -516,10 +516,8 @@ struct GRPCIdleHandlerStateMachine {
516516
case let .quiescing(state):
517517
let streamID = state.lastPeerInitiatedStreamID
518518
operations.sendGoAwayFrame(lastPeerInitiatedStreamID: streamID)
519-
case .operating, .waitingToIdle:
520-
// We can only ratchet down the stream ID if we're already quiescing.
521-
preconditionFailure()
522-
case .closing, .closed:
519+
case .operating, .waitingToIdle, .closing, .closed:
520+
// We can only need to ratchet down the stream ID if we're already quiescing.
523521
()
524522
}
525523

Sources/GRPC/Version.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ internal enum Version {
2222
internal static let minor = 7
2323

2424
/// The patch version.
25-
internal static let patch = 2
25+
internal static let patch = 3
2626

2727
/// The version string.
2828
internal static let versionString = "\(major).\(minor).\(patch)"

Tests/GRPCTests/GRPCIdleHandlerStateMachineTests.swift

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,29 @@ class GRPCIdleHandlerStateMachineTests: GRPCTestCase {
510510
let op8 = stateMachine.streamClosed(withID: 1)
511511
op8.assertShouldClose()
512512
}
513+
514+
func testRatchetDownStreamIDWhenNotQuiescing() {
515+
var stateMachine = self.makeServerStateMachine()
516+
_ = stateMachine.receiveSettings([])
517+
518+
// from the 'operating' state.
519+
stateMachine.ratchetDownGoAwayStreamID().assertDoNothing()
520+
521+
// move to the 'waiting to idle' state.
522+
let promise = EmbeddedEventLoop().makePromise(of: Void.self)
523+
let task = Scheduled(promise: promise, cancellationTask: {})
524+
stateMachine.scheduledIdleTimeoutTask(task).assertDoNothing()
525+
promise.succeed(())
526+
stateMachine.ratchetDownGoAwayStreamID().assertDoNothing()
527+
528+
// move to 'closing'
529+
_ = stateMachine.idleTimeoutTaskFired()
530+
stateMachine.ratchetDownGoAwayStreamID().assertDoNothing()
531+
532+
// move to 'closed'
533+
_ = stateMachine.channelInactive()
534+
stateMachine.ratchetDownGoAwayStreamID().assertDoNothing()
535+
}
513536
}
514537

515538
extension GRPCIdleHandlerStateMachine.Operations {

0 commit comments

Comments
 (0)