Repository: quic-go/quic-go
Pull request #4961 (2025-02-16 06:30:10)

refactor connection ID generator callback handling
Needed for #234.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4961?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 `86.66667%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 84.07%. Comparing base [(`d726a79`)](https://app.codecov.io/gh/quic-go/quic-go/commit/d726a795db8391d29b180f684f74b223d40471a1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`0357a2a`)](https://app.codecov.io/gh/quic-go/quic-go/commit/0357a2a0e9e158892b7ff9102df84f2e827d230d?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/4961?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 |
|---|---|---|
| [connection.go](https://app.codecov.io/gh/quic-go/quic-go/pull/4961?src=pr&el=tree&filepath=connection.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go#diff-Y29ubmVjdGlvbi5nbw==) | 50.00% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/quic-go/quic-go/pull/4961?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    #4961      +/-   ##
==========================================
- Coverage   84.21%   84.07%   -0.14%     
==========================================
  Files         152      149       -3     
  Lines       16617    16273     -344     
==========================================
- Hits        13993    13681     -312     
+ Misses       2099     2081      -18     
+ Partials      525      511      -14     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4961?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).


Patch:
diff --git a/conn_id_generator.go b/conn_id_generator.go
index c309c2cd..a5c203ff 100644
--- a/conn_id_generator.go
+++ b/conn_id_generator.go
@@ -8,41 +8,40 @@ import (
 	"github.com/quic-go/quic-go/internal/wire"
 )
 
+type connRunnerCallbacks struct {
+	AddConnectionID    func(protocol.ConnectionID)
+	RemoveConnectionID func(protocol.ConnectionID)
+	RetireConnectionID func(protocol.ConnectionID)
+	ReplaceWithClosed  func([]protocol.ConnectionID, []byte)
+}
+
 type connIDGenerator struct {
 	generator  ConnectionIDGenerator
+	connRunner connRunnerCallbacks
 	highestSeq uint64
 
 	activeSrcConnIDs        map[uint64]protocol.ConnectionID
 	initialClientDestConnID *protocol.ConnectionID // nil for the client
 
-	addConnectionID    func(protocol.ConnectionID)
-	statelessResetter  *statelessResetter
-	removeConnectionID func(protocol.ConnectionID)
-	retireConnectionID func(protocol.ConnectionID)
-	replaceWithClosed  func([]protocol.ConnectionID, []byte)
-	queueControlFrame  func(wire.Frame)
+	statelessResetter *statelessResetter
+
+	queueControlFrame func(wire.Frame)
 }
 
 func newConnIDGenerator(
 	initialConnectionID protocol.ConnectionID,
 	initialClientDestConnID *protocol.ConnectionID, // nil for the client
-	addConnectionID func(protocol.ConnectionID),
 	statelessResetter *statelessResetter,
-	removeConnectionID func(protocol.ConnectionID),
-	retireConnectionID func(protocol.ConnectionID),
-	replaceWithClosed func([]protocol.ConnectionID, []byte),
+	connRunner connRunnerCallbacks,
 	queueControlFrame func(wire.Frame),
 	generator ConnectionIDGenerator,
 ) *connIDGenerator {
 	m := &connIDGenerator{
-		generator:          generator,
-		activeSrcConnIDs:   make(map[uint64]protocol.ConnectionID),
-		addConnectionID:    addConnectionID,
-		statelessResetter:  statelessResetter,
-		removeConnectionID: removeConnectionID,
-		retireConnectionID: retireConnectionID,
-		replaceWithClosed:  replaceWithClosed,
-		queueControlFrame:  queueControlFrame,
+		generator:         generator,
+		activeSrcConnIDs:  make(map[uint64]protocol.ConnectionID),
+		statelessResetter: statelessResetter,
+		connRunner:        connRunner,
+		queueControlFrame: queueControlFrame,
 	}
 	m.activeSrcConnIDs[0] = initialConnectionID
 	m.initialClientDestConnID = initialClientDestConnID
@@ -85,7 +84,7 @@ func (m *connIDGenerator) Retire(seq uint64, sentWithDestConnID protocol.Connect
 			ErrorMessage: fmt.Sprintf("retired connection ID %d (%s), which was used as the Destination Connection ID on this packet", seq, connID),
 		}
 	}
-	m.retireConnectionID(connID)
+	m.connRunner.RetireConnectionID(connID)
 	delete(m.activeSrcConnIDs, seq)
 	// Don't issue a replacement for the initial connection ID.
 	if seq == 0 {
@@ -100,7 +99,7 @@ func (m *connIDGenerator) issueNewConnID() error {
 		return err
 	}
 	m.activeSrcConnIDs[m.highestSeq+1] = connID
-	m.addConnectionID(connID)
+	m.connRunner.AddConnectionID(connID)
 	m.queueControlFrame(&wire.NewConnectionIDFrame{
 		SequenceNumber:      m.highestSeq + 1,
 		ConnectionID:        connID,
@@ -112,17 +111,17 @@ func (m *connIDGenerator) issueNewConnID() error {
 
 func (m *connIDGenerator) SetHandshakeComplete() {
 	if m.initialClientDestConnID != nil {
-		m.retireConnectionID(*m.initialClientDestConnID)
+		m.connRunner.RetireConnectionID(*m.initialClientDestConnID)
 		m.initialClientDestConnID = nil
 	}
 }
 
 func (m *connIDGenerator) RemoveAll() {
 	if m.initialClientDestConnID != nil {
-		m.removeConnectionID(*m.initialClientDestConnID)
+		m.connRunner.RemoveConnectionID(*m.initialClientDestConnID)
 	}
 	for _, connID := range m.activeSrcConnIDs {
-		m.removeConnectionID(connID)
+		m.connRunner.RemoveConnectionID(connID)
 	}
 }
 
@@ -134,5 +133,5 @@ func (m *connIDGenerator) ReplaceWithClosed(connClose []byte) {
 	for _, connID := range m.activeSrcConnIDs {
 		connIDs = append(connIDs, connID)
 	}
-	m.replaceWithClosed(connIDs, connClose)
+	m.connRunner.ReplaceWithClosed(connIDs, connClose)
 }
diff --git a/connection.go b/connection.go
index 9415584d..5785c375 100644
--- a/connection.go
+++ b/connection.go
@@ -267,11 +267,13 @@ var newConnection = func(
 	s.connIDGenerator = newConnIDGenerator(
 		srcConnID,
 		&clientDestConnID,
-		func(connID protocol.ConnectionID) { runner.Add(connID, s) },
 		statelessResetter,
-		runner.Remove,
-		runner.Retire,
-		runner.ReplaceWithClosed,
+		connRunnerCallbacks{
+			AddConnectionID:    func(connID protocol.ConnectionID) { runner.Add(connID, s) },
+			RemoveConnectionID: runner.Remove,
+			RetireConnectionID: runner.Retire,
+			ReplaceWithClosed:  runner.ReplaceWithClosed,
+		},
 		s.queueControlFrame,
 		connIDGenerator,
 	)
@@ -378,11 +380,13 @@ var newClientConnection = func(
 	s.connIDGenerator = newConnIDGenerator(
 		srcConnID,
 		nil,
-		func(connID protocol.ConnectionID) { runner.Add(connID, s) },
 		statelessResetter,
-		runner.Remove,
-		runner.Retire,
-		runner.ReplaceWithClosed,
+		connRunnerCallbacks{
+			AddConnectionID:    func(connID protocol.ConnectionID) { runner.Add(connID, s) },
+			RemoveConnectionID: runner.Remove,
+			RetireConnectionID: runner.Retire,
+			ReplaceWithClosed:  runner.ReplaceWithClosed,
+		},
 		s.queueControlFrame,
 		connIDGenerator,
 	)


Test patch:
diff --git a/conn_id_generator_test.go b/conn_id_generator_test.go
index 85cc8215..4a9a0943 100644
--- a/conn_id_generator_test.go
+++ b/conn_id_generator_test.go
@@ -34,11 +34,13 @@ func testConnIDGeneratorIssueAndRetire(t *testing.T, hasInitialClientDestConnID
 	g := newConnIDGenerator(
 		protocol.ParseConnectionID([]byte{1, 1, 1, 1}),
 		initialClientDestConnID,
-		func(c protocol.ConnectionID) { added = append(added, c) },
 		sr,
-		func(c protocol.ConnectionID) { t.Fatal("didn't expect conn ID removals") },
-		func(c protocol.ConnectionID) { retired = append(retired, c) },
-		func([]protocol.ConnectionID, []byte) {},
+		connRunnerCallbacks{
+			AddConnectionID:    func(c protocol.ConnectionID) { added = append(added, c) },
+			RemoveConnectionID: func(c protocol.ConnectionID) { t.Fatal("didn't expect conn ID removals") },
+			RetireConnectionID: func(c protocol.ConnectionID) { retired = append(retired, c) },
+			ReplaceWithClosed:  func([]protocol.ConnectionID, []byte) {},
+		},
 		func(f wire.Frame) { queuedFrames = append(queuedFrames, f) },
 		&protocol.DefaultConnectionIDGenerator{ConnLen: 5},
 	)
@@ -116,11 +118,13 @@ func testConnIDGeneratorRemoveAll(t *testing.T, hasInitialClientDestConnID bool)
 	g := newConnIDGenerator(
 		protocol.ParseConnectionID([]byte{1, 1, 1, 1}),
 		initialClientDestConnID,
-		func(c protocol.ConnectionID) { added = append(added, c) },
 		newStatelessResetter(&StatelessResetKey{1, 2, 3, 4}),
-		func(c protocol.ConnectionID) { removed = append(removed, c) },
-		func(c protocol.ConnectionID) { t.Fatal("didn't expect conn ID retirements") },
-		func([]protocol.ConnectionID, []byte) {},
+		connRunnerCallbacks{
+			AddConnectionID:    func(c protocol.ConnectionID) { added = append(added, c) },
+			RemoveConnectionID: func(c protocol.ConnectionID) { removed = append(removed, c) },
+			RetireConnectionID: func(c protocol.ConnectionID) { t.Fatal("didn't expect conn ID retirements") },
+			ReplaceWithClosed:  func([]protocol.ConnectionID, []byte) {},
+		},
 		func(f wire.Frame) {},
 		&protocol.DefaultConnectionIDGenerator{ConnLen: 5},
 	)
@@ -164,13 +168,15 @@ func testConnIDGeneratorReplaceWithClosed(t *testing.T, hasInitialClientDestConn
 	g := newConnIDGenerator(
 		protocol.ParseConnectionID([]byte{1, 1, 1, 1}),
 		initialClientDestConnID,
-		func(c protocol.ConnectionID) { added = append(added, c) },
 		newStatelessResetter(&StatelessResetKey{1, 2, 3, 4}),
-		func(c protocol.ConnectionID) { t.Fatal("didn't expect conn ID removals") },
-		func(c protocol.ConnectionID) { t.Fatal("didn't expect conn ID retirements") },
-		func(connIDs []protocol.ConnectionID, b []byte) {
-			replaced = connIDs
-			replacedWith = b
+		connRunnerCallbacks{
+			AddConnectionID:    func(c protocol.ConnectionID) { added = append(added, c) },
+			RemoveConnectionID: func(c protocol.ConnectionID) { t.Fatal("didn't expect conn ID removals") },
+			RetireConnectionID: func(c protocol.ConnectionID) { t.Fatal("didn't expect conn ID retirements") },
+			ReplaceWithClosed: func(connIDs []protocol.ConnectionID, b []byte) {
+				replaced = connIDs
+				replacedWith = b
+			},
 		},
 		func(f wire.Frame) {},
 		&protocol.DefaultConnectionIDGenerator{ConnLen: 5},