wgengine/magicsock/tests: wait for home DERP connection before sending packets.

This fixes an elusive test flake. Fixes #161.

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>

wgengine/magicsock/magicsock.go

@@ -80,7 +80,7 @@ type Conn struct {
 
 	endpointsUpdateWaiter *sync.Cond
 	endpointsUpdateActive bool
-	wantEndpointsUpdate   string // non-empty for why reason
+	wantEndpointsUpdate   string // true if non-empty; string is reason
 	lastEndpoints         []string
 	peerSet               map[key.Public]struct{}
 
@@ -107,6 +107,7 @@ type Conn struct {
 	wantDerp      bool
 	privateKey    key.Private
 	myDerp        int // nearest DERP server; 0 means none/unknown
+	derpStarted   chan struct{}
 	activeDerp    map[int]activeDerp
 	prevDerp      map[int]*syncs.WaitGroupChan
 	derpTLSConfig *tls.Config // normally nil; used by tests
@@ -234,6 +235,7 @@ func Listen(opts Options) (*Conn, error) {
 		derpRecvCh:    make(chan derpReadResult),
 		udpRecvCh:     make(chan udpReadResult),
 		derpTLSConfig: opts.derpTLSConfig,
+		derpStarted:   make(chan struct{}),
 		derps:         opts.DERPs,
 		peerLastDerp:  make(map[key.Public]int),
 	}
@@ -271,6 +273,16 @@ func Listen(opts Options) (*Conn, error) {
 
 func (c *Conn) donec() <-chan struct{} { return c.connCtx.Done() }
 
+// WaitReady waits until the magicsock is entirely initialized and connected
+// to its home DERP server. This is normally not necessary, since magicsock
+// is intended to be entirely asynchronous, but it helps eliminate race
+// conditions in tests. In particular, you can't expect two test magicsocks
+// to be able to connect to each other through a test DERP unless they are
+// both fully initialized before you try.
+func (c *Conn) WaitReady() {
+	<-c.derpStarted
+}
+
 // ignoreSTUNPackets sets a STUN packet processing func that does nothing.
 func (c *Conn) ignoreSTUNPackets() {
 	c.stunReceiveFunc.Store(func([]byte, *net.UDPAddr) {})
@@ -834,7 +846,9 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr, peer key.Public) chan<- de
 	}
 	c.logf("magicsock: adding connection to derp-%v for %v", nodeID, why)
 
+	firstDerp := false
 	if c.activeDerp == nil {
+		firstDerp = true
 		c.activeDerp = make(map[int]activeDerp)
 		c.prevDerp = make(map[int]*syncs.WaitGroupChan)
 	}
@@ -880,7 +894,13 @@ func (c *Conn) derpWriteChanOfAddr(addr *net.UDPAddr, peer key.Public) chan<- de
 	wg.Add(2)
 	c.prevDerp[nodeID] = wg
 
-	go c.runDerpReader(ctx, addr, dc, wg, startGate)
+	go func() {
+		dc.Connect(ctx)
+		if firstDerp {
+			close(c.derpStarted)
+		}
+		c.runDerpReader(ctx, addr, dc, wg, startGate)
+	}()
 	go c.runDerpWriter(ctx, addr, dc, ch, wg, startGate)
 
 	return ad.writeCh

wgengine/magicsock/magicsock_test.go

@@ -406,11 +406,13 @@ func TestTwoDevicePing(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	ping1 := func(t *testing.T) {
-		t.Helper()
+	conn1.WaitReady()
+	conn2.WaitReady()
 
+	ping1 := func(t *testing.T) {
 		msg2to1 := tuntest.Ping(net.ParseIP("1.0.0.1"), net.ParseIP("1.0.0.2"))
 		tun2.Outbound <- msg2to1
+		t.Log("ping1 sent")
 		select {
 		case msgRecv := <-tun1.Inbound:
 			if !bytes.Equal(msg2to1, msgRecv) {
@@ -421,10 +423,9 @@ func TestTwoDevicePing(t *testing.T) {
 		}
 	}
 	ping2 := func(t *testing.T) {
-		t.Helper()
-
 		msg1to2 := tuntest.Ping(net.ParseIP("1.0.0.2"), net.ParseIP("1.0.0.1"))
 		tun1.Outbound <- msg1to2
+		t.Log("ping2 sent")
 		select {
 		case msgRecv := <-tun2.Inbound:
 			if !bytes.Equal(msg1to2, msgRecv) {
@@ -452,6 +453,7 @@ func TestTwoDevicePing(t *testing.T) {
 		if err := tstun1.InjectOutbound(msg1to2); err != nil {
 			t.Fatal(err)
 		}
+		t.Log("SendPacket sent")
 		select {
 		case msgRecv := <-tun2.Inbound:
 			if !bytes.Equal(msg1to2, msgRecv) {