diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index d177a01f4..66b55f1e2 100644 --- a/wgengine/magicsock/magicsock.go +++ b/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 diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 527766581..52e8f4b9c 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/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) {