Repository: quic-go/quic-go
Pull request #5116 (2025-05-04 04:35:31)

http3: reset request streams accepted after graceful shutdown
The server needs to let the client know that these streams are not getting processed, using the H3_REQUEST_REJECTED error code.

cc @WeidiDeng 

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/5116?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
Attention: Patch coverage is `42.85714%` with `16 lines` in your changes missing coverage. Please review.
> Project coverage is 83.96%. Comparing base [(`e1a1251`)](https://app.codecov.io/gh/quic-go/quic-go/commit/e1a1251c845ad322b72be9cb52b6cbb6883215be?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`d21d47a`)](https://app.codecov.io/gh/quic-go/quic-go/commit/d21d47a333b20a4095e65aa1a0450b4a0f834b45?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

| [Files with missing lines](https://app.codecov.io/gh/quic-go/quic-go/pull/5116?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) | Patch % | Lines |
|---|---|---|
| [http3/server.go](https://app.codecov.io/gh/quic-go/quic-go/pull/5116?src=pr&el=tree&filepath=http3%2Fserver.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go#diff-aHR0cDMvc2VydmVyLmdv) | 42.86% | [15 Missing and 1 partial :warning: ](https://app.codecov.io/gh/quic-go/quic-go/pull/5116?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #5116      +/-   ##
==========================================
- Coverage   84.17%   83.96%   -0.21%     
==========================================
  Files         153      150       -3     
  Lines       16992    16659     -333     
==========================================
- Hits        14302    13987     -315     
+ Misses       2161     2157       -4     
+ Partials      529      515      -14     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/5116?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/http3/server.go b/http3/server.go
index 88c255a9..10f3f80a 100644
--- a/http3/server.go
+++ b/http3/server.go
@@ -450,19 +450,19 @@ func (s *Server) handleConn(conn quic.Connection) error {
 	}).Append(b)
 	ctrlStr.Write(b)
 
-	ctx := conn.Context()
-	ctx = context.WithValue(ctx, ServerContextKey, s)
-	ctx = context.WithValue(ctx, http.LocalAddrContextKey, conn.LocalAddr())
-	ctx = context.WithValue(ctx, RemoteAddrContextKey, conn.RemoteAddr())
+	connCtx := conn.Context()
+	connCtx = context.WithValue(connCtx, ServerContextKey, s)
+	connCtx = context.WithValue(connCtx, http.LocalAddrContextKey, conn.LocalAddr())
+	connCtx = context.WithValue(connCtx, RemoteAddrContextKey, conn.RemoteAddr())
 	if s.ConnContext != nil {
-		ctx = s.ConnContext(ctx, conn)
-		if ctx == nil {
+		connCtx = s.ConnContext(connCtx, conn)
+		if connCtx == nil {
 			panic("http3: ConnContext returned nil")
 		}
 	}
 
 	hconn := newConnection(
-		ctx,
+		connCtx,
 		conn,
 		s.EnableDatagrams,
 		protocol.PerspectiveServer,
@@ -474,46 +474,57 @@ func (s *Server) handleConn(conn quic.Connection) error {
 	var nextStreamID quic.StreamID
 	var wg sync.WaitGroup
 	var handleErr error
+	var inGracefulShutdown bool
 	// Process all requests immediately.
 	// It's the client's responsibility to decide which requests are eligible for 0-RTT.
+	ctx := s.graceCtx
 	for {
-		str, datagrams, err := hconn.acceptStream(s.graceCtx)
+		// The context used here is:
+		// * before graceful shutdown: s.graceCtx
+		// * after graceful shutdown: s.closeCtx
+		// This allows us to keep accepting (and resetting) streams after graceful shutdown has started.
+		str, datagrams, err := hconn.acceptStream(ctx)
 		if err != nil {
+			// the underlying connection was closed (by either side)
+			if hconn.Context().Err() != nil {
+				var appErr *quic.ApplicationError
+				if !errors.As(err, &appErr) || appErr.ErrorCode != quic.ApplicationErrorCode(ErrCodeNoError) {
+					handleErr = fmt.Errorf("accepting stream failed: %w", err)
+				}
+				break
+			}
 			// server (not gracefully) closed, close the connection immediately
 			if s.closeCtx.Err() != nil {
 				conn.CloseWithError(quic.ApplicationErrorCode(ErrCodeNoError), "")
 				handleErr = http.ErrServerClosed
 				break
 			}
-
-			// gracefully closed, send GOAWAY frame and wait for requests to complete or grace period to end
-			// new requests will be rejected and shouldn't be sent
-			if s.graceCtx.Err() != nil {
-				wg.Add(1)
-				// Send the GOAWAY frame in a separate Goroutine.
-				// Sending might block if the peer didn't grant enough flow control credit.
-				// Write is guaranteed to return once the connection is closed.
-				go func() {
-					defer wg.Done()
-					_, _ = ctrlStr.Write((&goAwayFrame{StreamID: nextStreamID}).Append(nil))
-				}()
-
-				select {
-				case <-hconn.Context().Done():
-					// we expect the client to eventually close the connection after receiving the GOAWAY
-				case <-s.closeCtx.Done():
-					// close the connection after graceful period
-					conn.CloseWithError(quic.ApplicationErrorCode(ErrCodeNoError), "")
+			inGracefulShutdown = s.graceCtx.Err() != nil
+			if !inGracefulShutdown {
+				var appErr *quic.ApplicationError
+				if !errors.As(err, &appErr) || appErr.ErrorCode != quic.ApplicationErrorCode(ErrCodeNoError) {
+					handleErr = fmt.Errorf("accepting stream failed: %w", err)
 				}
-				handleErr = http.ErrServerClosed
 				break
 			}
 
-			var appErr *quic.ApplicationError
-			if !errors.As(err, &appErr) || appErr.ErrorCode != quic.ApplicationErrorCode(ErrCodeNoError) {
-				handleErr = fmt.Errorf("accepting stream failed: %w", err)
-			}
-			break
+			// gracefully closed, send GOAWAY frame and wait for requests to complete or grace period to end
+			// new requests will be rejected and shouldn't be sent
+			wg.Add(1)
+			// Send the GOAWAY frame in a separate Goroutine.
+			// Sending might block if the peer didn't grant enough flow control credit.
+			// Write is guaranteed to return once the connection is closed.
+			go func() {
+				defer wg.Done()
+				_, _ = ctrlStr.Write((&goAwayFrame{StreamID: nextStreamID}).Append(nil))
+			}()
+			ctx = s.closeCtx
+			continue
+		}
+		if inGracefulShutdown {
+			str.CancelRead(quic.StreamErrorCode(ErrCodeRequestRejected))
+			str.CancelWrite(quic.StreamErrorCode(ErrCodeRequestRejected))
+			continue
 		}
 
 		nextStreamID = str.StreamID() + 4
@@ -690,9 +701,10 @@ func (s *Server) Close() error {
 	return err
 }
 
-// Shutdown shuts down the server gracefully.
+// Shutdown gracefully shuts down the server without interrupting any active connections.
 // The server sends a GOAWAY frame first, then or for all running requests to complete.
-// Shutdown in combination with ListenAndServe() (instead of Serve()) may race if it is called before a UDP socket is established.
+// Shutdown in combination with ListenAndServe may race if it is called before a UDP socket is established.
+// It is recommended to use Serve instead.
 func (s *Server) Shutdown(ctx context.Context) error {
 	s.mutex.Lock()
 	s.closed = true


Test patch:
diff --git a/http3/server_test.go b/http3/server_test.go
index 1e33061a..b27d85f5 100644
--- a/http3/server_test.go
+++ b/http3/server_test.go
@@ -860,7 +860,7 @@ func TestServerClosing(t *testing.T) {
 	require.ErrorIs(t, s.ServeQUICConn(nil), http.ErrServerClosed)
 }
 
-func TestHandlesConcurrentServeAndClose(t *testing.T) {
+func TestServerConcurrentServeAndClose(t *testing.T) {
 	addr, err := net.ResolveUDPAddr("udp", "localhost:0")
 	require.NoError(t, err)
 	c, err := net.ListenUDP("udp", addr)
@@ -879,3 +879,134 @@ func TestHandlesConcurrentServeAndClose(t *testing.T) {
 		t.Fatal("timeout")
 	}
 }
+
+func TestServerGracefulShutdown(t *testing.T) {
+	s := &Server{TLSConfig: testdata.GetTLSConfig()}
+	s.init()
+
+	mockCtrl := gomock.NewController(t)
+	controlStr := mockquic.NewMockStream(mockCtrl)
+	controlStrChan := make(chan []byte, 1)
+	controlStr.EXPECT().Write(gomock.Any()).DoAndReturn(func(b []byte) (int, error) {
+		controlStrChan <- b
+		return len(b), nil
+	}).AnyTimes()
+
+	streamChan := make(chan quic.Stream, 1)
+	testDone := make(chan struct{})
+	conn := mockquic.NewMockEarlyConnection(mockCtrl)
+	conn.EXPECT().OpenUniStream().Return(controlStr, nil)
+	conn.EXPECT().AcceptUniStream(gomock.Any()).DoAndReturn(func(context.Context) (quic.ReceiveStream, error) {
+		<-testDone
+		return nil, assert.AnError
+	}).MaxTimes(1)
+	conn.EXPECT().AcceptStream(gomock.Any()).DoAndReturn(func(ctx context.Context) (quic.Stream, error) {
+		select {
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		case str, ok := <-streamChan:
+			if !ok {
+				return nil, assert.AnError
+			}
+			return str, nil
+		}
+	}).AnyTimes()
+	conn.EXPECT().RemoteAddr().Return(&net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 1337}).AnyTimes()
+	conn.EXPECT().LocalAddr().AnyTimes()
+	conn.EXPECT().Context().Return(context.Background()).AnyTimes()
+
+	firstStream := mockquic.NewMockStream(mockCtrl)
+	firstStreamAccepted := make(chan struct{}, 1)
+	firstStream.EXPECT().Read(gomock.Any()).DoAndReturn(func(b []byte) (int, error) {
+		firstStreamAccepted <- struct{}{}
+		<-testDone
+		return 0, assert.AnError
+	})
+	firstStream.EXPECT().StreamID().Return(quic.StreamID(1337)).AnyTimes()
+	firstStream.EXPECT().Context().Return(context.Background()).AnyTimes()
+	streamChan <- firstStream
+
+	go s.ServeQUICConn(conn)
+
+	var r bytes.Buffer
+	fp := &frameParser{r: &r}
+
+	select {
+	case b := <-controlStrChan:
+		_, l, err := quicvarint.Parse(b)
+		require.NoError(t, err)
+		r.Write(b[l:])
+		f, err := fp.ParseNext()
+		require.NoError(t, err)
+		require.IsType(t, &settingsFrame{}, f)
+	case <-time.After(time.Second):
+		t.Fatal("timeout")
+	}
+
+	select {
+	case <-firstStreamAccepted:
+	case <-time.After(time.Second):
+		t.Fatal("timeout")
+	}
+
+	time.Sleep(scaleDuration(10 * time.Millisecond))
+
+	ctx, cancel := context.WithCancel(context.Background())
+	errChan := make(chan error)
+	go func() {
+		errChan <- s.Shutdown(ctx)
+	}()
+
+	select {
+	case b := <-controlStrChan:
+		r.Write(b)
+		f, err := fp.ParseNext()
+		require.NoError(t, err)
+		require.Equal(t, &goAwayFrame{StreamID: 1337 + 4}, f)
+	case <-time.After(time.Second):
+		t.Fatal("timeout")
+	}
+
+	select {
+	case <-errChan:
+		t.Fatal("didn't expect Shutdown to return")
+	case <-time.After(scaleDuration(10 * time.Millisecond)):
+	}
+
+	// all further streams are getting rejected
+	for range 3 {
+		resetChan := make(chan struct{}, 2)
+		str := mockquic.NewMockStream(mockCtrl)
+		str.EXPECT().StreamID().AnyTimes()
+		str.EXPECT().Context().Return(context.Background()).AnyTimes()
+		str.EXPECT().CancelRead(quic.StreamErrorCode(ErrCodeRequestRejected)).Do(func(sec quic.StreamErrorCode) {
+			resetChan <- struct{}{}
+		})
+		str.EXPECT().CancelWrite(quic.StreamErrorCode(ErrCodeRequestRejected)).Do(func(sec quic.StreamErrorCode) {
+			resetChan <- struct{}{}
+		})
+		streamChan <- str
+
+		for range 2 {
+			select {
+			case <-resetChan:
+			case <-time.After(time.Second):
+				t.Fatal("expected stream reset")
+			}
+		}
+	}
+
+	// cancel the context passed to Shutdown
+	conn.EXPECT().CloseWithError(quic.ApplicationErrorCode(ErrCodeNoError), gomock.Any())
+	cancel()
+	firstStream.EXPECT().CancelRead(gomock.Any())
+	firstStream.EXPECT().CancelWrite(gomock.Any())
+	close(testDone)
+
+	select {
+	case err := <-errChan:
+		require.ErrorIs(t, err, context.Canceled)
+	case <-time.After(time.Second):
+		t.Fatal("timeout")
+	}
+}
diff --git a/integrationtests/self/http_shutdown_test.go b/integrationtests/self/http_shutdown_test.go
index 8d677182..fd1e00e2 100644
--- a/integrationtests/self/http_shutdown_test.go
+++ b/integrationtests/self/http_shutdown_test.go
@@ -4,11 +4,13 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"testing"
 	"time"
 
 	"github.com/quic-go/quic-go/http3"
+	quicproxy "github.com/quic-go/quic-go/integrationtests/tools/proxy"
 
 	"github.com/stretchr/testify/require"
 )
@@ -23,7 +25,7 @@ func TestHTTPShutdown(t *testing.T) {
 		go func() {
 			require.NoError(t, server.Close())
 		}()
-		time.Sleep(50 * time.Millisecond) // make sure the server started shutting down
+		time.Sleep(scaleDuration(10 * time.Millisecond)) // make sure the server started shutting down
 	})
 
 	_, err := client.Get(fmt.Sprintf("https://localhost:%d/shutdown", port))
@@ -34,7 +36,7 @@ func TestHTTPShutdown(t *testing.T) {
 }
 
 func TestGracefulShutdownShortRequest(t *testing.T) {
-	delay := scaleDuration(50 * time.Millisecond)
+	delay := scaleDuration(25 * time.Millisecond)
 
 	var server *http3.Server
 	mux := http.NewServeMux()
@@ -71,7 +73,7 @@ func TestGracefulShutdownShortRequest(t *testing.T) {
 }
 
 func TestGracefulShutdownLongLivedRequest(t *testing.T) {
-	delay := scaleDuration(50 * time.Millisecond)
+	delay := scaleDuration(25 * time.Millisecond)
 	errChan := make(chan error, 1)
 	requestChan := make(chan time.Duration, 1)
 
@@ -125,3 +127,72 @@ func TestGracefulShutdownLongLivedRequest(t *testing.T) {
 		t.Fatal("did not receive request duration")
 	}
 }
+
+func TestGracefulShutdownPendingStreams(t *testing.T) {
+	rtt := scaleDuration(25 * time.Millisecond)
+
+	handlerChan := make(chan struct{}, 1)
+	mux := http.NewServeMux()
+	mux.HandleFunc("/helloworld", func(w http.ResponseWriter, r *http.Request) {
+		handlerChan <- struct{}{}
+		time.Sleep(rtt)
+		w.Write([]byte("hello world"))
+	})
+	var server *http3.Server
+	port := startHTTPServer(t, mux, func(s *http3.Server) { server = s })
+	client := newHTTP3Client(t)
+
+	proxy := quicproxy.Proxy{
+		Conn:       newUDPConnLocalhost(t),
+		ServerAddr: &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: port},
+		DelayPacket: func(_ quicproxy.Direction, _, _ net.Addr, data []byte) time.Duration {
+			return rtt
+		},
+	}
+	require.NoError(t, proxy.Start())
+	defer proxy.Close()
+	proxyPort := proxy.LocalAddr().(*net.UDPAddr).Port
+
+	errChan := make(chan error, 1)
+	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://localhost:%d/helloworld", proxyPort), nil)
+	require.NoError(t, err)
+	go func() {
+		resp, err := client.Do(req)
+		if err != nil {
+			errChan <- err
+			return
+		}
+		if resp.StatusCode != http.StatusOK {
+			errChan <- fmt.Errorf("expected status code %d, got %d", http.StatusOK, resp.StatusCode)
+		}
+	}()
+
+	select {
+	case <-handlerChan:
+	case <-time.After(time.Second):
+		t.Fatal("did not receive request")
+	}
+
+	shutdownChan := make(chan error, 1)
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() { shutdownChan <- server.Shutdown(ctx) }()
+	time.Sleep(rtt / 2) // wait for the server to start shutting down
+
+	// make sure that the server rejects further requests
+	for range 3 {
+		req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("https://localhost:%d/helloworld", proxyPort), nil)
+		require.NoError(t, err)
+		_, err = client.Do(req)
+		var h3err *http3.Error
+		require.ErrorAs(t, err, &h3err)
+		require.Equal(t, http3.ErrCodeRequestRejected, h3err.ErrorCode)
+	}
+
+	cancel()
+	select {
+	case err := <-shutdownChan:
+		require.ErrorIs(t, err, context.Canceled)
+	case <-time.After(time.Second):
+		t.Fatal("shutdown did not complete")
+	}
+}