Remove the circular reference

Author: nbrownus

control.go

@@ -53,7 +53,7 @@ type Control struct {
 	statsStart             func()
 	dnsStart               func()
 	lighthouseStart        func()
-	networkChangeStart     func()
+	networkChangeStart     func(rebind func())
 	connectionManagerStart func(context.Context)
 }
 
@@ -106,7 +106,7 @@ func (c *Control) Start() error {
 		go c.dnsStart()
 	}
 	if c.networkChangeStart != nil {
-		go c.networkChangeStart()
+		go c.networkChangeStart(c.RebindUDPServer)
 	}
 	if c.connectionManagerStart != nil {
 		go c.connectionManagerStart(c.ctx)

main.go

@@ -268,7 +268,9 @@ func Main(c *config.C, configTest bool, buildVersion string, l *slog.Logger, dev
 
 	attachCommands(l, c, ssh, ifce)
 
-	control := &Control{
+	networkChanges := udp.NewNetworkChangeMonitor(ctx, l, c)
+
+	return &Control{
 		state:                  StateReady,
 		f:                      ifce,
 		l:                      l,
@@ -278,15 +280,9 @@ func Main(c *config.C, configTest bool, buildVersion string, l *slog.Logger, dev
 		statsStart:             stats.Start,
 		dnsStart:               ds.Start,
 		lighthouseStart:        lightHouse.StartUpdateWorker,
+		networkChangeStart:     networkChanges.Start,
 		connectionManagerStart: connManager.Start,
-	}
-
-	// The monitor's whole job is to trigger a rebind, and Control is what owns rebinding and the state gating
-	// around it, so it is injected the rebind func and can only be built once Control exists.
-	networkChanges := udp.NewNetworkChangeMonitor(ctx, l, c, control.RebindUDPServer)
-	control.networkChangeStart = networkChanges.Start
-
-	return control, nil
+	}, nil
 }
 
 func moduleVersion() string {

udp/netchange.go

@@ -11,35 +11,30 @@ import (
 //
 // Detection lives here in the udp package, next to the socket it concerns and the platform matrix that already knows
 // which sockets go stale. What to do about a change — updating the lighthouse, requerying tunnels — is not the udp
-// package's business, so the reaction is injected as a plain function pointer. The monitor calls it once a change
-// has settled and stays ignorant of what it does.
+// package's business, so Start takes the reaction as a plain function. Passing it at Start rather than holding it
+// keeps this package from referencing whatever owns the rebind.
 //
 // On platforms whose sockets do not go stale, watchNetworkChanges hands back a nil channel and Start returns.
 type NetworkChangeMonitor struct {
 	l       *slog.Logger
 	ctx     context.Context
 	enabled bool
-	// rebind is what we call once a change has settled. It is injected because the udp package has no business
-	// knowing what a rebind entails, only when one is warranted.
-	rebind func()
 }
 
-// NewNetworkChangeMonitor builds a monitor that calls rebind whenever the local network moves. The returned monitor
-// is always usable: Start is safe to call unconditionally, it no-ops when disabled or on a platform that does not
-// need it.
-func NewNetworkChangeMonitor(ctx context.Context, l *slog.Logger, c *config.C, rebind func()) *NetworkChangeMonitor {
+// NewNetworkChangeMonitor builds a monitor for local network changes. The returned monitor is always usable: Start
+// is safe to call unconditionally, it no-ops when disabled or on a platform that does not need it.
+func NewNetworkChangeMonitor(ctx context.Context, l *slog.Logger, c *config.C) *NetworkChangeMonitor {
 	return &NetworkChangeMonitor{
 		l:       l,
 		ctx:     ctx,
 		enabled: c.GetBool("listen.rebind_on_network_change", true),
-		rebind:  rebind,
 	}
 }
 
 // Start watches for network changes until the context is cancelled, calling rebind once per settled change. It
 // blocks, so callers run it in a goroutine, and it no-ops when disabled, unsupported, or with nothing to rebind.
-func (m *NetworkChangeMonitor) Start() {
-	if !m.enabled || m.rebind == nil || m.ctx.Err() != nil {
+func (m *NetworkChangeMonitor) Start(rebind func()) {
+	if !m.enabled || rebind == nil || m.ctx.Err() != nil {
 		return
 	}
 
@@ -61,6 +56,6 @@ func (m *NetworkChangeMonitor) Start() {
 
 	for range changes {
 		m.l.Info("Local network changed, rebinding the udp listener")
-		m.rebind()
+		rebind()
 	}
 }

udp/netchange_darwin_test.go

@@ -216,11 +216,11 @@ func TestNetworkChangeMonitorStopsWithContext(t *testing.T) {
 	l := test.NewLogger()
 	c := config.NewC(l)
 	require.NoError(t, c.LoadString("listen:\n  rebind_on_network_change: true\n"))
-	m := NewNetworkChangeMonitor(ctx, l, c, func() {})
+	m := NewNetworkChangeMonitor(ctx, l, c)
 
 	done := make(chan struct{})
 	go func() {
-		m.Start()
+		m.Start(func() {})
 		close(done)
 	}()
 
@@ -240,5 +240,5 @@ func TestNetworkChangeMonitorStopsWithContext(t *testing.T) {
 	}
 
 	// Starting again after the context is dead must not open anything.
-	m.Start()
+	m.Start(func() {})
 }

udp/netchange_test.go

@@ -10,30 +10,30 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func newMonitor(t *testing.T, ctx context.Context, cfg string, rebind func()) *NetworkChangeMonitor {
+func newMonitor(t *testing.T, ctx context.Context, cfg string) *NetworkChangeMonitor {
 	t.Helper()
 	l := test.NewLogger()
 	c := config.NewC(l)
 	require.NoError(t, c.LoadString(cfg))
-	return NewNetworkChangeMonitor(ctx, l, c, rebind)
+	return NewNetworkChangeMonitor(ctx, l, c)
 }
 
 func TestNetworkChangeMonitorDefaultsOn(t *testing.T) {
 	// Says nothing about rebinding, so this covers the default.
-	m := newMonitor(t, context.Background(), "listen:\n  host: 0.0.0.0\n", func() {})
+	m := newMonitor(t, context.Background(), "listen:\n  host: 0.0.0.0\n")
 	assert.True(t, m.enabled, "should default to on")
 }
 
 func TestNetworkChangeMonitorDisabledIsANoOp(t *testing.T) {
-	m := newMonitor(t, context.Background(), "listen:\n  rebind_on_network_change: false\n", func() {})
+	m := newMonitor(t, context.Background(), "listen:\n  rebind_on_network_change: false\n")
 	require.False(t, m.enabled)
 
 	// Must return without opening a socket. If it watched anything this would block.
-	m.Start()
+	m.Start(func() {})
 }
 
 func TestNetworkChangeMonitorNilRebindIsANoOp(t *testing.T) {
 	// Nothing to rebind, so there is no point watching, on any platform.
-	m := newMonitor(t, context.Background(), "listen:\n  rebind_on_network_change: true\n", nil)
-	m.Start()
+	m := newMonitor(t, context.Background(), "listen:\n  rebind_on_network_change: true\n")
+	m.Start(nil)
 }