Skip to content

HTTP2: Add more GOAWAY tests #28443

@geoffkizer

Description

@geoffkizer

I'm adding some basic GOAWAY tests in dotnet/corefx#34630. However, there are some more scenarios we should test here.

Scenario 1: GOAWAY with unprocessed streams

When the server sends a GOAWAY with a last stream ID that's less than an existing pending request stream that the client has already sent, the client should retry those requests on a new connection.

Scenario 2: GOAWAY causes new requests to use a new connection

When the server sends a GOAWAY, but is still processing outstanding requests, any new client request should cause a new connection to be established, even though the old connection is still processing outstanding requests.

Note this is somewhat tricky to implement in the current code, because the Http2LoopbackServer assumes there is only one active connection at a time. We will need to refactor this somehow.

It's also tricky because there's a timing issue about when the client processes the GOAWAY. There's no "ack" for a GOAWAY frame, so we can't know for sure that the client has actually processed it yet. Possibly we could finesse this by sending an empty SETTINGS after the GOAWAY, and waiting for an ACK on this, before initiating a new client request.

EDIT (@krwq)
this should also address @geoffkizer's comment dotnet/corefx#39036 (comment)

BTW, this should also (hopefully) fix some related issues with GOAWAY. Since we are currently disposing when we receive GOAWAY, we disallow any frames from being sent after GOAWAY is received. This affect PING per dotnet/corefx#38558, but it also affects a whole bunch of other cases as well, such as:

(1) A request body may still be in flight, and as long as the GOAWAY last stream id indicates it's valid, we need to continue to send it
(2) Outbound WINDOW_UPDATE for response bodies
(3) SETTINGS ACK, if we receive a new SETTINGS frame (unlikely, but still valid, I think)

It would be nice to have tests for at least some of this, but it's not necessary as part of this PR.

Metadata

Metadata

Assignees

Labels

test-enhancementImprovements of test source code

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions