From da90ea664d7a601a73cd531bc5ae1db0993033c1 Mon Sep 17 00:00:00 2001
From: Fernando Serboncini <fserb@tailscale.com>
Date: Thu, 26 Feb 2026 12:36:26 -0500
Subject: [PATCH] wgengine/magicsock: only run derpActiveFunc after connecting
 to DERP (#18814)

derpActiveFunc was being called immediately as a bare goroutine,
before startGate was resolved. For the firstDerp case, startGate
is c.derpStarted which only closes after dc.Connect() completes,
so derpActiveFunc was firing before the DERP connection existed.

We now block it with the same logic used by runDerpReader and by
runDerpWriter.

Updates: #18810

Signed-off-by: Fernando Serboncini <fserb@tailscale.com>
---
 wgengine/magicsock/derp.go           |  9 ++++-
 wgengine/magicsock/magicsock_test.go | 51 ++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go
index b3cc5c2ce..f9e505070 100644
--- a/wgengine/magicsock/derp.go
+++ b/wgengine/magicsock/derp.go
@@ -436,7 +436,14 @@ func (c *Conn) derpWriteChanForRegion(regionID int, peer key.NodePublic) chan de
 
 	go c.runDerpReader(ctx, regionID, dc, wg, startGate)
 	go c.runDerpWriter(ctx, dc, ch, wg, startGate)
-	go c.derpActiveFunc()
+	go func() {
+		select {
+		case <-ctx.Done():
+			return
+		case <-startGate:
+			c.derpActiveFunc()
+		}
+	}()
 
 	return ad.writeCh
 }
diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go
index 5fa177b3b..9d6cae87b 100644
--- a/wgengine/magicsock/magicsock_test.go
+++ b/wgengine/magicsock/magicsock_test.go
@@ -530,6 +530,57 @@ func TestPickDERPFallback(t *testing.T) {
 	// have fixed DERP fallback logic.
 }
 
+// TestDERPActiveFuncCalledAfterConnect verifies that DERPActiveFunc is not
+// called until the DERP connection is actually established (i.e. after
+// startGate / derpStarted is closed).
+func TestDERPActiveFuncCalledAfterConnect(t *testing.T) {
+	derpMap, cleanup := runDERPAndStun(t, t.Logf, localhostListener{}, netaddr.IPv4(127, 0, 0, 1))
+	defer cleanup()
+
+	bus := eventbustest.NewBus(t)
+
+	netMon, err := netmon.New(bus, t.Logf)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	resultCh := make(chan bool, 1)
+	var conn *Conn
+
+	conn, err = NewConn(Options{
+		Logf:                   t.Logf,
+		NetMon:                 netMon,
+		EventBus:               bus,
+		HealthTracker:          health.NewTracker(bus),
+		Metrics:                new(usermetric.Registry),
+		DisablePortMapper:      true,
+		TestOnlyPacketListener: localhostListener{},
+		EndpointsFunc:          func([]tailcfg.Endpoint) {},
+		DERPActiveFunc: func() {
+			// derpStarted should already be closed when DERPActiveFunc is called.
+			select {
+			case <-conn.derpStarted:
+				resultCh <- true
+			default:
+				resultCh <- false
+			}
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+
+	conn.SetDERPMap(derpMap)
+	if err := conn.SetPrivateKey(key.NewNode()); err != nil {
+		t.Fatal(err)
+	}
+
+	if ok := <-resultCh; !ok {
+		t.Error("DERPActiveFunc was called before DERP connection was established")
+	}
+}
+
 // TestDeviceStartStop exercises the startup and shutdown logic of
 // wireguard-go, which is intimately intertwined with magicsock's own
 // lifecycle. We seem to be good at generating deadlocks here, so if