Skip to content

Commit

Permalink
[client] Check wginterface instead of engine ctx (#2676)
Browse files Browse the repository at this point in the history
Moving code to ensure wgInterface is gone right after context is
cancelled/stop in the off chance that on next retry the backoff
operation is permanently cancelled and interface is abandoned without
destroying.
  • Loading branch information
hurricanehrndz authored Oct 4, 2024
1 parent 5897a48 commit f603cd9
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 18 deletions.
15 changes: 9 additions & 6 deletions client/internal/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,6 @@ func (c *ConnectClient) run(
checks := loginResp.GetChecks()

c.engineMutex.Lock()
if c.engine != nil && c.engine.ctx.Err() != nil {
log.Info("Stopping Netbird Engine")
if err := c.engine.Stop(); err != nil {
log.Errorf("Failed to stop engine: %v", err)
}
}
c.engine = NewEngineWithProbes(engineCtx, cancel, signalClient, mgmClient, relayManager, engineConfig, mobileDependency, c.statusRecorder, probes, checks)

c.engineMutex.Unlock()
Expand All @@ -294,6 +288,15 @@ func (c *ConnectClient) run(
}

<-engineCtx.Done()
c.engineMutex.Lock()
if c.engine != nil && c.engine.wgInterface != nil {
log.Infof("ensuring %s is removed, Netbird engine context cancelled", c.engine.wgInterface.Name())
if err := c.engine.Stop(); err != nil {
log.Errorf("Failed to stop engine: %v", err)
}
c.engine = nil
}
c.engineMutex.Unlock()
c.statusRecorder.ClientTeardown()

backOff.Reset()
Expand Down
26 changes: 14 additions & 12 deletions client/internal/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,13 @@ func (e *Engine) Stop() error {
}
log.Info("Network monitor: stopped")

// stop/restore DNS first so dbus and friends don't complain because of a missing interface
e.stopDNSServer()

if e.routeManager != nil {
e.routeManager.Stop()
}

err := e.removeAllPeers()
if err != nil {
return fmt.Errorf("failed to remove all peers: %s", err)
Expand Down Expand Up @@ -1116,18 +1123,12 @@ func (e *Engine) close() {
}
}

// stop/restore DNS first so dbus and friends don't complain because of a missing interface
e.stopDNSServer()

if e.routeManager != nil {
e.routeManager.Stop()
}

log.Debugf("removing Netbird interface %s", e.config.WgIfaceName)
if e.wgInterface != nil {
if err := e.wgInterface.Close(); err != nil {
log.Errorf("failed closing Netbird interface %s %v", e.config.WgIfaceName, err)
}
e.wgInterface = nil
}

if !isNil(e.sshServer) {
Expand Down Expand Up @@ -1395,7 +1396,7 @@ func (e *Engine) startNetworkMonitor() {
}

// Set a new timer to debounce rapid network changes
debounceTimer = time.AfterFunc(1*time.Second, func() {
debounceTimer = time.AfterFunc(2*time.Second, func() {
// This function is called after the debounce period
mu.Lock()
defer mu.Unlock()
Expand Down Expand Up @@ -1426,17 +1427,18 @@ func (e *Engine) addrViaRoutes(addr netip.Addr) (bool, netip.Prefix, error) {
}

func (e *Engine) stopDNSServer() {
if e.dnsServer == nil {
return
}
e.dnsServer.Stop()
e.dnsServer = nil
err := fmt.Errorf("DNS server stopped")
nsGroupStates := e.statusRecorder.GetDNSStates()
for i := range nsGroupStates {
nsGroupStates[i].Enabled = false
nsGroupStates[i].Error = err
}
e.statusRecorder.UpdateDNSStates(nsGroupStates)
if e.dnsServer != nil {
e.dnsServer.Stop()
e.dnsServer = nil
}
}

// isChecksEqual checks if two slices of checks are equal.
Expand Down

0 comments on commit f603cd9

Please sign in to comment.