Skip to content

Commit

Permalink
Ignore state machine inputs in unexpected states (#1374)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
glbrntt authored Mar 25, 2022
1 parent adb32bf commit 593fe0f
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 5 deletions.
6 changes: 2 additions & 4 deletions Sources/GRPC/GRPCIdleHandlerStateMachine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -516,10 +516,8 @@ struct GRPCIdleHandlerStateMachine {
case let .quiescing(state):
let streamID = state.lastPeerInitiatedStreamID
operations.sendGoAwayFrame(lastPeerInitiatedStreamID: streamID)
case .operating, .waitingToIdle:
// We can only ratchet down the stream ID if we're already quiescing.
preconditionFailure()
case .closing, .closed:
case .operating, .waitingToIdle, .closing, .closed:
// We can only need to ratchet down the stream ID if we're already quiescing.
()
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/GRPC/Version.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ internal enum Version {
internal static let minor = 7

/// The patch version.
internal static let patch = 2
internal static let patch = 3

/// The version string.
internal static let versionString = "\(major).\(minor).\(patch)"
Expand Down
23 changes: 23 additions & 0 deletions Tests/GRPCTests/GRPCIdleHandlerStateMachineTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,29 @@ class GRPCIdleHandlerStateMachineTests: GRPCTestCase {
let op8 = stateMachine.streamClosed(withID: 1)
op8.assertShouldClose()
}

func testRatchetDownStreamIDWhenNotQuiescing() {
var stateMachine = self.makeServerStateMachine()
_ = stateMachine.receiveSettings([])

// from the 'operating' state.
stateMachine.ratchetDownGoAwayStreamID().assertDoNothing()

// move to the 'waiting to idle' state.
let promise = EmbeddedEventLoop().makePromise(of: Void.self)
let task = Scheduled(promise: promise, cancellationTask: {})
stateMachine.scheduledIdleTimeoutTask(task).assertDoNothing()
promise.succeed(())
stateMachine.ratchetDownGoAwayStreamID().assertDoNothing()

// move to 'closing'
_ = stateMachine.idleTimeoutTaskFired()
stateMachine.ratchetDownGoAwayStreamID().assertDoNothing()

// move to 'closed'
_ = stateMachine.channelInactive()
stateMachine.ratchetDownGoAwayStreamID().assertDoNothing()
}
}

extension GRPCIdleHandlerStateMachine.Operations {
Expand Down

0 comments on commit 593fe0f

Please sign in to comment.