Repository: quic-go/quic-go
Pull request #5121 (2025-05-05 11:18:04)

fix race condition in tests when setting the key update interval

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/5121?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 `88.88889%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 84.02%. Comparing base [(`57e46f8`)](https://app.codecov.io/gh/quic-go/quic-go/commit/57e46f8a4c1c2ebe8703aa275514d5c7324b3de4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`2373886`)](https://app.codecov.io/gh/quic-go/quic-go/commit/2373886996da0b47a81f07d8630847e1e27caf17?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/5121?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 |
|---|---|---|
| [internal/handshake/updatable\_aead.go](https://app.codecov.io/gh/quic-go/quic-go/pull/5121?src=pr&el=tree&filepath=internal%2Fhandshake%2Fupdatable_aead.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go#diff-aW50ZXJuYWwvaGFuZHNoYWtlL3VwZGF0YWJsZV9hZWFkLmdv) | 88.89% | [1 Missing :warning: ](https://app.codecov.io/gh/quic-go/quic-go/pull/5121?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    #5121      +/-   ##
==========================================
- Coverage   84.22%   84.02%   -0.20%     
==========================================
  Files         153      150       -3     
  Lines       17207    16869     -338     
==========================================
- Hits        14492    14174     -318     
+ Misses       2164     2154      -10     
+ Partials      551      541      -10     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/5121?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/internal/handshake/updatable_aead.go b/internal/handshake/updatable_aead.go
index ceaa8047..0aca46fe 100644
--- a/internal/handshake/updatable_aead.go
+++ b/internal/handshake/updatable_aead.go
@@ -6,6 +6,9 @@ import (
 	"crypto/tls"
 	"encoding/binary"
 	"fmt"
+	"os"
+	"strconv"
+	"testing"
 	"time"
 
 	"github.com/quic-go/quic-go/internal/protocol"
@@ -14,9 +17,15 @@ import (
 	"github.com/quic-go/quic-go/logging"
 )
 
-// KeyUpdateInterval is the maximum number of packets we send or receive before initiating a key update.
-// It's a package-level variable to allow modifying it for testing purposes.
-var KeyUpdateInterval uint64 = protocol.KeyUpdateInterval
+func keyUpdateInterval() uint64 {
+	// Reparsing the environment variable is not very performant, but it's only done in tests.
+	if testing.Testing() {
+		if v, err := strconv.ParseUint(os.Getenv("QUIC_GO_TEST_KEY_UPDATE_INTERVAL"), 10, 64); err == nil {
+			return v
+		}
+	}
+	return protocol.KeyUpdateInterval
+}
 
 // FirstKeyUpdateInterval is the maximum number of packets we send or receive before initiating the first key update.
 // It's a package-level variable to allow modifying it for testing purposes.
@@ -293,11 +302,11 @@ func (a *updatableAEAD) shouldInitiateKeyUpdate() bool {
 			return true
 		}
 	}
-	if a.numRcvdWithCurrentKey >= KeyUpdateInterval {
+	if a.numRcvdWithCurrentKey >= keyUpdateInterval() {
 		a.logger.Debugf("Received %d packets with current key phase. Initiating key update to the next key phase: %d", a.numRcvdWithCurrentKey, a.keyPhase+1)
 		return true
 	}
-	if a.numSentWithCurrentKey >= KeyUpdateInterval {
+	if a.numSentWithCurrentKey >= keyUpdateInterval() {
 		a.logger.Debugf("Sent %d packets with current key phase. Initiating key update to the next key phase: %d", a.numSentWithCurrentKey, a.keyPhase+1)
 		return true
 	}


Test patch:
diff --git a/integrationtests/self/key_update_test.go b/integrationtests/self/key_update_test.go
index ae72299e..d70c1a8e 100644
--- a/integrationtests/self/key_update_test.go
+++ b/integrationtests/self/key_update_test.go
@@ -7,7 +7,6 @@ import (
 	"time"
 
 	"github.com/quic-go/quic-go"
-	"github.com/quic-go/quic-go/internal/handshake"
 	"github.com/quic-go/quic-go/internal/protocol"
 	"github.com/quic-go/quic-go/logging"
 
@@ -15,9 +14,7 @@ import (
 )
 
 func TestKeyUpdates(t *testing.T) {
-	origKeyUpdateInterval := handshake.KeyUpdateInterval
-	t.Cleanup(func() { handshake.KeyUpdateInterval = origKeyUpdateInterval })
-	handshake.KeyUpdateInterval = 1 // update keys as frequently as possible
+	t.Setenv("QUIC_GO_TEST_KEY_UPDATE_INTERVAL", "1") // update keys as frequently as possible
 
 	var sentHeaders []*logging.ShortHeader
 	var receivedHeaders []*logging.ShortHeader
diff --git a/integrationtests/self/self_test.go b/integrationtests/self/self_test.go
index 7516cafc..6df7863b 100644
--- a/integrationtests/self/self_test.go
+++ b/integrationtests/self/self_test.go
@@ -1,7 +1,6 @@
 package self_test
 
 import (
-	"bytes"
 	"context"
 	"crypto/tls"
 	"crypto/x509"
@@ -11,9 +10,7 @@ import (
 	"math/rand/v2"
 	"net"
 	"os"
-	"runtime/pprof"
 	"strconv"
-	"strings"
 	"testing"
 	"time"
 
@@ -161,12 +158,6 @@ func newUDPConnLocalhost(t testing.TB) *net.UDPConn {
 	return conn
 }
 
-func areTransportsRunning() bool {
-	var b bytes.Buffer
-	pprof.Lookup("goroutine").WriteTo(&b, 1)
-	return strings.Contains(b.String(), "quic-go.(*Transport).listen")
-}
-
 func TestMain(m *testing.M) {
 	var versionParam string
 	flag.StringVar(&versionParam, "version", "1", "QUIC version")
@@ -184,15 +175,7 @@ func TestMain(m *testing.M) {
 	}
 	fmt.Printf("using QUIC version: %s\n", version)
 
-	status := m.Run()
-	if status != 0 {
-		os.Exit(status)
-	}
-	if areTransportsRunning() {
-		fmt.Println("stray transport goroutines found")
-		os.Exit(1)
-	}
-	os.Exit(status)
+	os.Exit(m.Run())
 }
 
 func scaleDuration(d time.Duration) time.Duration {
diff --git a/internal/handshake/updatable_aead_test.go b/internal/handshake/updatable_aead_test.go
index 2b4cefa9..36e0638e 100644
--- a/internal/handshake/updatable_aead_test.go
+++ b/internal/handshake/updatable_aead_test.go
@@ -330,15 +330,12 @@ func TestRejectFrequentKeyUpdates(t *testing.T) {
 }
 
 func setKeyUpdateIntervals(t *testing.T, firstKeyUpdateInterval, keyUpdateInterval uint64) {
-	origKeyUpdateInterval := KeyUpdateInterval
+	t.Setenv("QUIC_GO_TEST_KEY_UPDATE_INTERVAL", fmt.Sprintf("%d", keyUpdateInterval))
+
 	origFirstKeyUpdateInterval := FirstKeyUpdateInterval
-	KeyUpdateInterval = keyUpdateInterval
 	FirstKeyUpdateInterval = firstKeyUpdateInterval
 
-	t.Cleanup(func() {
-		KeyUpdateInterval = origKeyUpdateInterval
-		FirstKeyUpdateInterval = origFirstKeyUpdateInterval
-	})
+	t.Cleanup(func() { FirstKeyUpdateInterval = origFirstKeyUpdateInterval })
 }
 
 func TestInitiateKeyUpdateAfterSendingMaxPackets(t *testing.T) {
@@ -382,7 +379,7 @@ func TestInitiateKeyUpdateAfterSendingMaxPackets(t *testing.T) {
 
 func TestKeyUpdateEnforceACKKeyPhase(t *testing.T) {
 	const firstKeyUpdateInterval = 5
-	setKeyUpdateIntervals(t, firstKeyUpdateInterval, KeyUpdateInterval)
+	setKeyUpdateIntervals(t, firstKeyUpdateInterval, protocol.KeyUpdateInterval)
 
 	_, server, serverTracer := setupEndpoints(t, &utils.RTTStats{})
 	server.SetHandshakeConfirmed()