From d7c1ecce92ae0a9d0f994ec71380743749385a2e Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@tailscale.com>
Date: Tue, 25 Aug 2026 09:44:03 +0000
Subject: [PATCH] mapper: correct the send timer rationale

time.After has been GC-recoverable since Go 1.23, so the leak the comment
described cannot happen and the test asserting it proved nothing.
---
 hscontrol/mapper/batcher_unit_test.go | 45 +--------------------------
 hscontrol/mapper/node_conn.go         |  6 ++--
 2 files changed, 3 insertions(+), 48 deletions(-)

diff --git a/hscontrol/mapper/batcher_unit_test.go b/hscontrol/mapper/batcher_unit_test.go
index bef897fe4..08d515cdd 100644
--- a/hscontrol/mapper/batcher_unit_test.go
+++ b/hscontrol/mapper/batcher_unit_test.go
@@ -950,52 +950,9 @@ func TestMultiChannelSend_ConcurrentRemoveAndSend(t *testing.T) {
 }
 
 // ============================================================================
-// Regression tests for H1 (timer leak) and H3 (lifecycle)
+// Regression test for H3 (lifecycle)
 // ============================================================================
 
-// TestConnectionEntry_SendFastPath_TimerStopped is a regression guard for H1.
-// Before the fix, connectionEntry.send used time.After(50ms) which leaked a
-// timer into the runtime heap on every call even when the channel send
-// succeeded immediately. The fix switched to time.NewTimer + defer Stop().
-//
-// This test sends many messages on a buffered (non-blocking) channel and
-// checks that the number of live goroutines stays bounded, which would
-// grow without bound under the old time.After approach at high call rates.
-func TestConnectionEntry_SendFastPath_TimerStopped(t *testing.T) {
-	const sends = 5000
-
-	ch := make(chan *tailcfg.MapResponse, sends)
-
-	entry := &connectionEntry{
-		id:      "timer-leak-test",
-		c:       ch,
-		version: 100,
-		created: time.Now(),
-	}
-
-	resp := testMapResponse()
-
-	for range sends {
-		err := entry.send(resp)
-		require.NoError(t, err)
-	}
-
-	// Drain the channel so we aren't holding references.
-	for range sends {
-		<-ch
-	}
-
-	// Force a GC + timer cleanup pass.
-	runtime.GC()
-
-	// If timers were leaking we'd see a goroutine count much higher
-	// than baseline. With 5000 leaked timers the count would be
-	// noticeably elevated. We just check it's reasonable.
-	numGR := runtime.NumGoroutine()
-	assert.Less(t, numGR, 200,
-		"goroutine count after %d fast-path sends should be bounded; got %d (possible timer leak)", sends, numGR)
-}
-
 // TestBatcher_CloseWaitsForWorkers is a regression guard for H3.
 // Before the fix, Close() would tear down node connections while workers
 // were potentially still running, risking sends on closed channels.
diff --git a/hscontrol/mapper/node_conn.go b/hscontrol/mapper/node_conn.go
index 97d024a85..acbab5361 100644
--- a/hscontrol/mapper/node_conn.go
+++ b/hscontrol/mapper/node_conn.go
@@ -403,10 +403,8 @@ func (entry *connectionEntry) send(data *tailcfg.MapResponse) error {
 	// This is critical for detecting Docker containers that are forcefully terminated
 	// but still have channels that appear open.
 	//
-	// We use time.NewTimer + Stop instead of time.After to avoid leaking timers.
-	// time.After creates a timer that lives in the runtime's timer heap until it fires,
-	// even when the send succeeds immediately. On the hot path (1000+ nodes per tick),
-	// this leaks thousands of timers per second.
+	// Use a timer rather than time.After so the timeout is explicitly released
+	// on the fast path; both are GC-recoverable since Go 1.23.
 	timer := time.NewTimer(50 * time.Millisecond) //nolint:mnd
 	defer timer.Stop()