Skip to content

Conversation

@zarqman
Copy link
Contributor

@zarqman zarqman commented Dec 7, 2025

This PR changes the behavior upon a second GOAWAY on a connection. Before, it raised a ProtocolError. After, the GOAWAY is allowed with no further action.

Duplicate GOAWAYs have been seen irl with dnsdist, which uses the nghttp2 library.

Per rfc9113, section 6.8, GOAWAY may be resent:

  • if there is a delay between an earlier GOAWAY and closing the socket

    A GOAWAY frame might not immediately precede closing of the connection; a
    receiver of a GOAWAY that has no more use for the connection SHOULD still send a GOAWAY frame before terminating the connection.

  • to send different values

    An endpoint MAY send multiple GOAWAY frames if circumstances change.

I considered letting :goaway fall through to the else condition and raising after a 15 second delay. However, I believe 15 seconds would still be too small to satisfy conditions of an intentional delay between first and second GOAWAY frames.

For example, a server could send GOAWAY to indicate no new streams, but continue to finish existing streams which could easily take longer than 15 seconds. When finished, the server is likely to send a second GOAWAY when preparing to close the socket. This scenario would be normal behavior for a server performing a graceful shutdown with medium-to-long running requests.

Per rfc9113, section 6.8, GOAWAY may be resent:
- if there is a delay between an earlier GOAWAY and closing the socket
- to send different values

> A GOAWAY frame might not immediately precede closing of the connection; a
  receiver of a GOAWAY that has no more use for the connection SHOULD still
  send a GOAWAY frame before terminating the connection.

> An endpoint MAY send multiple GOAWAY frames if circumstances change.
@HoneyryderChuck HoneyryderChuck force-pushed the allow-multiple-goaway branch 2 times, most recently from 23ad643 to e337700 Compare December 9, 2025 12:00
@HoneyryderChuck
Copy link
Collaborator

thx @zarqman

@HoneyryderChuck HoneyryderChuck merged commit 39ec88d into igrigorik:main Dec 9, 2025
8 of 9 checks passed
@zarqman zarqman deleted the allow-multiple-goaway branch December 9, 2025 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants