Repository: quic-go/quic-go
Pull request #5108 (2025-05-03 09:23:40)

never close accepted connections when the listener is closed
The `Transport` now shuts itself down automatically once it doesn't track any more connections (it waits for the expiry of closed connections as well).

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/5108?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 83.90%. Comparing base [(`d7b8447`)](https://app.codecov.io/gh/quic-go/quic-go/commit/d7b8447e0e9ab65cb4bc14e276a156a727a7bba1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`019ec25`)](https://app.codecov.io/gh/quic-go/quic-go/commit/019ec25de747e16b63a982a4a3544b6d3027a62a?dropdown=coverage&el=desc&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    #5108      +/-   ##
==========================================
- Coverage   84.01%   83.90%   -0.11%     
==========================================
  Files         153      150       -3     
  Lines       16996    16651     -345     
==========================================
- Hits        14279    13971     -308     
+ Misses       2185     2164      -21     
+ Partials      532      516      -16     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/5108?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/server.go b/server.go
index fae34be2..e328521d 100644
--- a/server.go
+++ b/server.go
@@ -140,9 +140,7 @@ func (l *Listener) Accept(ctx context.Context) (Connection, error) {
 // Close closes the listener.
 // Accept will return [ErrServerClosed] as soon as all connections in the accept queue have been accepted.
 // QUIC handshakes that are still in flight will be rejected with a CONNECTION_REFUSED error.
-// The effect of closing the listener depends on how it was created:
-//   - if it was created using [Transport.Listen], already established connections will be unaffected
-//   - if it was created using the [Listen] convenience method, all established connection will be closed immediately
+// Already established (accepted)connections will be unaffected.
 func (l *Listener) Close() error {
 	return l.baseServer.Close()
 }
@@ -721,7 +719,7 @@ func (s *baseServer) handleInitialImpl(p receivedPacket, hdr *wire.Header) error
 	// The only time this collision will occur if we receive the two Initial packets at the same time.
 	if added := s.tr.AddWithConnID(hdr.DestConnectionID, connID, conn); !added {
 		delete(s.zeroRTTQueues, hdr.DestConnectionID)
-		conn.closeWithTransportError(qerr.ConnectionRefused)
+		conn.closeWithTransportError(ConnectionRefused)
 		return nil
 	}
 	// Pass queued 0-RTT to the newly established connection.
@@ -845,7 +843,7 @@ func (s *baseServer) maybeSendInvalidToken(p rejectedPacket) {
 	if s.logger.Debug() {
 		s.logger.Debugf("Client sent an invalid retry token. Sending INVALID_TOKEN to %s.", p.remoteAddr)
 	}
-	if err := s.sendError(p.remoteAddr, hdr, sealer, qerr.InvalidToken, p.info); err != nil {
+	if err := s.sendError(p.remoteAddr, hdr, sealer, InvalidToken, p.info); err != nil {
 		s.logger.Debugf("Error sending INVALID_TOKEN error: %s", err)
 	}
 }
@@ -853,7 +851,7 @@ func (s *baseServer) maybeSendInvalidToken(p rejectedPacket) {
 func (s *baseServer) sendConnectionRefused(p rejectedPacket) {
 	defer p.buffer.Release()
 	sealer, _ := handshake.NewInitialAEAD(p.hdr.DestConnectionID, protocol.PerspectiveServer, p.hdr.Version)
-	if err := s.sendError(p.remoteAddr, p.hdr, sealer, qerr.ConnectionRefused, p.info); err != nil {
+	if err := s.sendError(p.remoteAddr, p.hdr, sealer, ConnectionRefused, p.info); err != nil {
 		s.logger.Debugf("Error sending CONNECTION_REFUSED error: %s", err)
 	}
 }
diff --git a/transport.go b/transport.go
index 527bdf00..65a2d803 100644
--- a/transport.go
+++ b/transport.go
@@ -422,7 +422,14 @@ func (t *Transport) init(allowZeroLengthConnIDs bool) error {
 		}
 		t.statelessResetter = newStatelessResetter(t.StatelessResetKey)
 
-		go t.listen(conn)
+		go func() {
+			defer close(t.listening)
+			t.listen(conn)
+
+			if t.createdConn {
+				conn.Close()
+			}
+		}()
 		go t.runSendQueue()
 	})
 	return t.initErr
@@ -473,18 +480,17 @@ func (t *Transport) Close() error {
 
 func (t *Transport) closeServer() {
 	t.mutex.Lock()
+	defer t.mutex.Unlock()
+
 	t.server = nil
 	if t.isSingleUse {
 		t.closeErr = ErrServerClosed
 	}
-	t.mutex.Unlock()
-	if t.createdConn {
-		t.Conn.Close()
-	}
-	if t.isSingleUse {
-		t.conn.SetReadDeadline(time.Now())
-		defer func() { t.conn.SetReadDeadline(time.Time{}) }()
-		<-t.listening // wait until listening returns
+
+	t.connMx.Lock()
+	defer t.connMx.Unlock()
+	if len(t.handlers) == 0 {
+		t.maybeStopListening()
 	}
 }
 
@@ -523,8 +529,6 @@ func (t *Transport) close(e error) {
 var setBufferWarningOnce sync.Once
 
 func (t *Transport) listen(conn rawConn) {
-	defer close(t.listening)
-
 	for {
 		p, err := conn.ReadPacket()
 		//nolint:staticcheck // SA1019 ignore this!
@@ -553,6 +557,12 @@ func (t *Transport) listen(conn rawConn) {
 	}
 }
 
+func (t *Transport) maybeStopListening() {
+	if t.isSingleUse && t.closeErr != nil {
+		t.conn.SetReadDeadline(time.Now())
+	}
+}
+
 func (t *Transport) handlePacket(p receivedPacket) {
 	if len(p.data) == 0 {
 		return
@@ -819,6 +829,12 @@ func (h *packetHandlerMap) ReplaceWithClosed(ids []protocol.ConnectionID, connCl
 		for _, id := range ids {
 			delete(h.handlers, id)
 		}
+		if len(h.handlers) == 0 {
+			t := (*Transport)(h)
+			t.mutex.Lock()
+			t.maybeStopListening()
+			t.mutex.Unlock()
+		}
 		h.connMx.Unlock()
 		h.logger.Debugf("Removing connection IDs %s for a closed connection after it has been retired.", ids)
 	})


Test patch:
diff --git a/integrationtests/self/handshake_test.go b/integrationtests/self/handshake_test.go
index 50cc743a..f3b9230c 100644
--- a/integrationtests/self/handshake_test.go
+++ b/integrationtests/self/handshake_test.go
@@ -7,7 +7,9 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"runtime"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -339,53 +341,123 @@ func TestServerAcceptQueueOverflow(t *testing.T) {
 	require.Equal(t, quic.ConnectionRefused, transportErr.ErrorCode)
 }
 
-func TestHandshakingConnectionsClosedOnServerShutdown(t *testing.T) {
-	tr := &quic.Transport{Conn: newUDPConnLocalhost(t)}
-	addTracer(tr)
-	defer tr.Close()
+func TestHandshakeCloseListener(t *testing.T) {
+	t.Run("using Transport.Listen", func(t *testing.T) {
+		testHandshakeCloseListener(t, func(tlsConf *tls.Config) *quic.Listener {
+			tr := &quic.Transport{Conn: newUDPConnLocalhost(t)}
+			addTracer(tr)
+			t.Cleanup(func() { tr.Close() })
+
+			ln, err := tr.Listen(tlsConf, getQuicConfig(nil))
+			require.NoError(t, err)
+			return ln
+		})
+	})
+
+	t.Run("using Listen", func(t *testing.T) {
+		conn := newUDPConnLocalhost(t)
+		testHandshakeCloseListener(t, func(tlsConf *tls.Config) *quic.Listener {
+			ln, err := quic.Listen(conn, tlsConf, getQuicConfig(nil))
+			require.NoError(t, err)
+			return ln
+		})
+
+		// make sure that the Transport didn't close the underlying connection
+		conn2 := newUDPConnLocalhost(t)
+		_, err := conn2.WriteTo([]byte("test"), conn2.LocalAddr())
+		require.NoError(t, err)
 
-	rtt := scaleDuration(40 * time.Millisecond)
+		conn2.SetReadDeadline(time.Now().Add(time.Second))
+		b := make([]byte, 1000)
+		n, err := conn2.Read(b)
+		require.NoError(t, err)
+		require.Equal(t, "test", string(b[:n]))
+	})
+
+	// This test is somewhat slow (600ms), since the connection entries are kept for 3 PTOs.
+	t.Run("using ListenAddr", func(t *testing.T) {
+		var lnAddr *net.UDPAddr
+		testHandshakeCloseListener(t, func(tlsConf *tls.Config) *quic.Listener {
+			ln, err := quic.ListenAddr("127.0.0.1:0", tlsConf, getQuicConfig(nil))
+			require.NoError(t, err)
+			lnAddr = ln.Addr().(*net.UDPAddr)
+			return ln
+		})
+
+		// make sure that the Transport closed the underlying connection
+		if runtime.GOOS != "windows" { // this check doesn't work on Windows
+			require.Eventually(t, func() bool {
+				conn, err := net.DialUDP("udp", nil, lnAddr)
+				require.NoError(t, err)
+				defer conn.Close()
+				_, err = conn.Write([]byte("test"))
+				require.NoError(t, err)
+				conn.SetReadDeadline(time.Now().Add(scaleDuration(10 * time.Millisecond)))
+				_, err = conn.Read(make([]byte, 1000))
+				require.Error(t, err)
+				return strings.Contains(err.Error(), "read: connection refused")
+			}, time.Second, 50*time.Millisecond)
+		}
+	})
+}
+
+func testHandshakeCloseListener(t *testing.T, createListener func(*tls.Config) *quic.Listener) {
 	connQueued := make(chan struct{})
+	var sawFirst atomic.Bool
 	tlsConf := &tls.Config{
-		GetConfigForClient: func(*tls.ClientHelloInfo) (*tls.Config, error) {
-			close(connQueued)
-			// Sleep for a bit.
-			// This allows the server to close the connection before the handshake completes.
-			time.Sleep(rtt / 2)
+		GetConfigForClient: func(info *tls.ClientHelloInfo) (*tls.Config, error) {
+			isFirst := sawFirst.CompareAndSwap(false, true)
+			if isFirst {
+			} else {
+				// Sleep for a bit.
+				// This allows the server to close the connection before the handshake completes.
+				close(connQueued)
+				time.Sleep(scaleDuration(10 * time.Millisecond))
+			}
 			return getTLSConfig(), nil
 		},
 	}
 
-	ln, err := tr.Listen(tlsConf, getQuicConfig(nil))
-	require.NoError(t, err)
+	ln := createListener(tlsConf)
 
-	proxy := quicproxy.Proxy{
-		Conn:        newUDPConnLocalhost(t),
-		ServerAddr:  ln.Addr().(*net.UDPAddr),
-		DelayPacket: func(quicproxy.Direction, net.Addr, net.Addr, []byte) time.Duration { return rtt / 2 },
-	}
-	require.NoError(t, proxy.Start())
-	defer proxy.Close()
+	// dial the first connection
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+	conn, err := quic.Dial(ctx, newUDPConnLocalhost(t), ln.Addr(), getTLSClientConfig(), getQuicConfig(nil))
+	require.NoError(t, err)
+	defer conn.CloseWithError(0, "")
 
 	errChan := make(chan error, 1)
-	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-	defer cancel()
 	go func() {
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
 		_, err := quic.Dial(ctx, newUDPConnLocalhost(t), ln.Addr(), getTLSClientConfig(), getQuicConfig(nil))
 		errChan <- err
 	}()
 
 	select {
 	case <-connQueued:
-	case <-time.After(5 * rtt):
+	case <-time.After(scaleDuration(10 * time.Millisecond)):
 		t.Fatal("timeout waiting for connection queued")
 	}
+
 	require.NoError(t, ln.Close())
 
-	err = <-errChan
-	var transportErr *quic.TransportError
-	require.ErrorAs(t, err, &transportErr)
-	require.Equal(t, quic.ConnectionRefused, transportErr.ErrorCode)
+	select {
+	case err := <-errChan:
+		var transportErr *quic.TransportError
+		require.ErrorAs(t, err, &transportErr)
+		require.Equal(t, quic.ConnectionRefused, transportErr.ErrorCode)
+	case <-time.After(time.Second):
+		t.Fatal("timeout waiting for handshaking connection to be rejected")
+	}
+
+	// the first connection should not be closed
+	select {
+	case <-conn.Context().Done():
+		t.Fatal("connection was closed")
+	case <-time.After(scaleDuration(10 * time.Millisecond)):
+	}
 }
 
 func TestALPN(t *testing.T) {