Merge branch 'main' into fix/ios-common-tunnel-notifier

Close previously returned immediately after marking the notifier
closed, so deliverLoop could still be invoking listener/DNS callbacks
after Close returned. Close now blocks on a completion channel that
deliverLoop closes once it has drained the queue and exited.

client/internal/connect.go

@@ -34,6 +34,7 @@ import (
 	"github.com/netbirdio/netbird/client/internal/profilemanager"
 	"github.com/netbirdio/netbird/client/internal/statemanager"
 	"github.com/netbirdio/netbird/client/internal/stdnet"
+	"github.com/netbirdio/netbird/client/internal/tunnelnotifier"
 	"github.com/netbirdio/netbird/client/internal/updater"
 	"github.com/netbirdio/netbird/client/internal/updater/installer"
 	nbnet "github.com/netbirdio/netbird/client/net"
@@ -136,10 +137,13 @@ func (c *ConnectClient) RunOniOS(
 	// Set GC percent to 5% to reduce memory usage as iOS only allows 50MB of memory for the extension.
 	debug.SetGCPercent(5)
 
+	notifier := tunnelnotifier.New(networkChangeListener, dnsManager)
+	defer notifier.Close()
+
 	mobileDependency := MobileDependency{
 		FileDescriptor:        fileDescriptor,
-		NetworkChangeListener: networkChangeListener,
-		DnsManager:            dnsManager,
+		NetworkChangeListener: notifier,
+		DnsManager:            notifier,
 		StateFilePath:         stateFilePath,
 		TempDir:               cacheDir,
 	}

client/internal/mobile_dependency.go

@@ -11,12 +11,14 @@ import (
 
 // MobileDependency collect all dependencies for mobile platform
 type MobileDependency struct {
-	// Android only
-	TunAdapter            device.TunAdapter
-	IFaceDiscover         stdnet.ExternalIFaceDiscover
+	// Android and iOS
 	NetworkChangeListener listener.NetworkChangeListener
-	HostDNSAddresses      []netip.AddrPort
-	DnsReadyListener      dns.ReadyListener
+
+	// Android only
+	TunAdapter       device.TunAdapter
+	IFaceDiscover    stdnet.ExternalIFaceDiscover
+	HostDNSAddresses []netip.AddrPort
+	DnsReadyListener dns.ReadyListener
 
 	//	iOS only
 	DnsManager     dns.IosDnsManager

client/internal/routemanager/notifier/notifier_ios.go

@@ -3,7 +3,6 @@
 package notifier
 
 import (
-	"container/list"
 	"net/netip"
 	"slices"
 	"sort"
@@ -16,20 +15,12 @@ import (
 
 type Notifier struct {
 	mu              sync.Mutex
-	cond            *sync.Cond
 	currentPrefixes []string
 	listener        listener.NetworkChangeListener
-	queue           *list.List
-	closed          bool
 }
 
 func NewNotifier() *Notifier {
-	n := &Notifier{
-		queue: list.New(),
-	}
-	n.cond = sync.NewCond(&n.mu)
-	go n.deliverLoop()
-	return n
+	return &Notifier{}
 }
 
 func (n *Notifier) SetListener(listener listener.NetworkChangeListener) {
@@ -59,44 +50,19 @@ func (n *Notifier) OnNewPrefixes(prefixes []netip.Prefix) {
 	sort.Strings(newNets)
 
 	n.mu.Lock()
+	defer n.mu.Unlock()
 	if slices.Equal(n.currentPrefixes, newNets) {
-		n.mu.Unlock()
 		return
 	}
 	n.currentPrefixes = newNets
-	routes := strings.Join(n.currentPrefixes, ",")
-	n.queue.PushBack(routes)
-	n.cond.Signal()
-	n.mu.Unlock()
+	if n.listener != nil {
+		n.listener.OnNetworkChanged(strings.Join(n.currentPrefixes, ","))
+	}
 }
 
 func (n *Notifier) Close() {
-	n.mu.Lock()
-	n.closed = true
-	n.cond.Signal()
-	n.mu.Unlock()
 }
 
 func (n *Notifier) GetInitialRouteRanges() []string {
 	return nil
 }
-
-func (n *Notifier) deliverLoop() {
-	for {
-		n.mu.Lock()
-		for n.queue.Len() == 0 && !n.closed {
-			n.cond.Wait()
-		}
-		if n.closed && n.queue.Len() == 0 {
-			n.mu.Unlock()
-			return
-		}
-		routes := n.queue.Remove(n.queue.Front()).(string)
-		l := n.listener
-		n.mu.Unlock()
-
-		if l != nil {
-			l.OnNetworkChanged(routes)
-		}
-	}
-}

client/internal/tunnelnotifier/notifier.go

@@ -0,0 +1,124 @@
+package tunnelnotifier
+
+import (
+	"container/list"
+	"sync"
+
+	"github.com/netbirdio/netbird/client/internal/dns"
+	"github.com/netbirdio/netbird/client/internal/listener"
+)
+
+type eventKind int
+
+const (
+	eventRoutes eventKind = iota
+	eventIfaceIP
+	eventIfaceIPv6
+	eventDNS
+)
+
+var (
+	_ listener.NetworkChangeListener = (*Notifier)(nil)
+	_ dns.IosDnsManager              = (*Notifier)(nil)
+)
+
+type event struct {
+	kind    eventKind
+	payload string
+}
+
+type Notifier struct {
+	mu     sync.Mutex
+	cond   *sync.Cond
+	queue  *list.List
+	closed bool
+	done   chan struct{}
+
+	listener   listener.NetworkChangeListener
+	dnsManager dns.IosDnsManager
+}
+
+func New(l listener.NetworkChangeListener, dm dns.IosDnsManager) *Notifier {
+	n := &Notifier{
+		queue:      list.New(),
+		done:       make(chan struct{}),
+		listener:   l,
+		dnsManager: dm,
+	}
+	n.cond = sync.NewCond(&n.mu)
+	go n.deliverLoop()
+	return n
+}
+
+func (n *Notifier) OnNetworkChanged(routes string) {
+	n.enqueue(event{kind: eventRoutes, payload: routes})
+}
+
+func (n *Notifier) SetInterfaceIP(ip string) {
+	n.enqueue(event{kind: eventIfaceIP, payload: ip})
+}
+
+func (n *Notifier) SetInterfaceIPv6(ip string) {
+	n.enqueue(event{kind: eventIfaceIPv6, payload: ip})
+}
+
+func (n *Notifier) ApplyDns(config string) {
+	n.enqueue(event{kind: eventDNS, payload: config})
+}
+
+// Close stops accepting new events and blocks until the delivery loop has
+// drained all queued events and exited.
+func (n *Notifier) Close() {
+	n.mu.Lock()
+	n.closed = true
+	n.cond.Signal()
+	n.mu.Unlock()
+	<-n.done
+}
+
+func (n *Notifier) enqueue(ev event) {
+	n.mu.Lock()
+	defer n.mu.Unlock()
+	if n.closed {
+		return
+	}
+	n.queue.PushBack(ev)
+	n.cond.Signal()
+}
+
+func (n *Notifier) deliverLoop() {
+	defer close(n.done)
+	for {
+		n.mu.Lock()
+		for n.queue.Len() == 0 && !n.closed {
+			n.cond.Wait()
+		}
+		if n.closed && n.queue.Len() == 0 {
+			n.mu.Unlock()
+			return
+		}
+		ev := n.queue.Remove(n.queue.Front()).(event)
+		l := n.listener
+		dm := n.dnsManager
+		n.mu.Unlock()
+
+		switch ev.kind {
+		case eventRoutes:
+			if l != nil {
+				l.OnNetworkChanged(ev.payload)
+			}
+		case eventIfaceIP:
+			if l != nil {
+				l.SetInterfaceIP(ev.payload)
+			}
+		case eventIfaceIPv6:
+			if l != nil {
+				l.SetInterfaceIPv6(ev.payload)
+			}
+		case eventDNS:
+			if dm != nil {
+				dm.ApplyDns(ev.payload)
+			}
+		}
+	}
+}

client/internal/tunnelnotifier/notifier_test.go

@@ -0,0 +1,192 @@
+package tunnelnotifier
+
+import (
+	"fmt"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type call struct {
+	kind    string
+	payload string
+}
+
+type recorder struct {
+	mu       sync.Mutex
+	calls    []call
+	inFlight atomic.Int32
+	overlap  atomic.Bool
+	delay    time.Duration
+}
+
+func (r *recorder) record(kind, payload string) {
+	if r.inFlight.Add(1) != 1 {
+		r.overlap.Store(true)
+	}
+	if r.delay > 0 {
+		time.Sleep(r.delay)
+	}
+	r.mu.Lock()
+	r.calls = append(r.calls, call{kind: kind, payload: payload})
+	r.mu.Unlock()
+	r.inFlight.Add(-1)
+}
+
+func (r *recorder) count() int {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	return len(r.calls)
+}
+
+func (r *recorder) snapshot() []call {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	out := make([]call, len(r.calls))
+	copy(out, r.calls)
+	return out
+}
+
+type fakeListener struct {
+	rec *recorder
+}
+
+func (f *fakeListener) OnNetworkChanged(routes string) {
+	f.rec.record("routes", routes)
+}
+
+func (f *fakeListener) SetInterfaceIP(ip string) {
+	f.rec.record("ip", ip)
+}
+
+func (f *fakeListener) SetInterfaceIPv6(ip string) {
+	f.rec.record("ipv6", ip)
+}
+
+type fakeDNSManager struct {
+	rec *recorder
+}
+
+func (f *fakeDNSManager) ApplyDns(config string) {
+	f.rec.record("dns", config)
+}
+
+func TestFIFOOrder(t *testing.T) {
+	rec := &recorder{}
+	n := New(&fakeListener{rec: rec}, &fakeDNSManager{rec: rec})
+	defer n.Close()
+
+	n.SetInterfaceIP("10.0.0.1")
+	n.SetInterfaceIPv6("fd00::1")
+	n.ApplyDns(`{"domains":[]}`)
+	n.OnNetworkChanged("10.0.0.0/8,192.168.0.0/16")
+	n.ApplyDns(`{"domains":["example.com"]}`)
+
+	require.Eventually(t, func() bool { return rec.count() == 5 }, time.Second, time.Millisecond)
+
+	expected := []call{
+		{kind: "ip", payload: "10.0.0.1"},
+		{kind: "ipv6", payload: "fd00::1"},
+		{kind: "dns", payload: `{"domains":[]}`},
+		{kind: "routes", payload: "10.0.0.0/8,192.168.0.0/16"},
+		{kind: "dns", payload: `{"domains":["example.com"]}`},
+	}
+	assert.Equal(t, expected, rec.snapshot())
+}
+
+func TestNoOverlappingCalls(t *testing.T) {
+	rec := &recorder{delay: 100 * time.Microsecond}
+	n := New(&fakeListener{rec: rec}, &fakeDNSManager{rec: rec})
+	defer n.Close()
+
+	const producers = 8
+	const perProducer = 25
+
+	var wg sync.WaitGroup
+	for i := 0; i < producers; i++ {
+		wg.Add(1)
+		go func(id int) {
+			defer wg.Done()
+			for j := 0; j < perProducer; j++ {
+				payload := fmt.Sprintf("%d-%d", id, j)
+				switch j % 4 {
+				case 0:
+					n.OnNetworkChanged(payload)
+				case 1:
+					n.SetInterfaceIP(payload)
+				case 2:
+					n.SetInterfaceIPv6(payload)
+				case 3:
+					n.ApplyDns(payload)
+				}
+			}
+		}(i)
+	}
+	wg.Wait()
+
+	require.Eventually(t, func() bool { return rec.count() == producers*perProducer }, 5*time.Second, time.Millisecond)
+	assert.False(t, rec.overlap.Load())
+}
+
+func TestDNSAndRoutesInterleaved(t *testing.T) {
+	rec := &recorder{delay: 100 * time.Microsecond}
+	n := New(&fakeListener{rec: rec}, &fakeDNSManager{rec: rec})
+	defer n.Close()
+
+	const events = 50
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+	go func() {
+		defer wg.Done()
+		for i := 0; i < events; i++ {
+			n.ApplyDns(fmt.Sprintf("dns-%d", i))
+		}
+	}()
+	go func() {
+		defer wg.Done()
+		for i := 0; i < events; i++ {
+			n.OnNetworkChanged(fmt.Sprintf("routes-%d", i))
+		}
+	}()
+	wg.Wait()
+
+	require.Eventually(t, func() bool { return rec.count() == 2*events }, 5*time.Second, time.Millisecond)
+	assert.False(t, rec.overlap.Load())
+
+	var dnsSeen, routesSeen int
+	for _, c := range rec.snapshot() {
+		switch c.kind {
+		case "dns":
+			assert.Equal(t, fmt.Sprintf("dns-%d", dnsSeen), c.payload)
+			dnsSeen++
+		case "routes":
+			assert.Equal(t, fmt.Sprintf("routes-%d", routesSeen), c.payload)
+			routesSeen++
+		}
+	}
+	assert.Equal(t, events, dnsSeen)
+	assert.Equal(t, events, routesSeen)
+}
+
+func TestCloseDrainsQueue(t *testing.T) {
+	rec := &recorder{delay: time.Millisecond}
+	n := New(&fakeListener{rec: rec}, &fakeDNSManager{rec: rec})
+
+	const events = 20
+	for i := 0; i < events; i++ {
+		n.OnNetworkChanged(fmt.Sprintf("routes-%d", i))
+	}
+	n.Close()
+
+	require.Equal(t, events, rec.count(), "Close must not return before all queued events are delivered")
+
+	n.OnNetworkChanged("after-close")
+	n.ApplyDns("after-close")
+	time.Sleep(50 * time.Millisecond)
+	assert.Equal(t, events, rec.count())
+}