From 60a6506c08c784c8b13659b4318af8e3c7e788aa Mon Sep 17 00:00:00 2001 From: abhishek9686 Date: Wed, 18 Mar 2026 17:19:39 +0530 Subject: [PATCH 1/2] NM-279: fix random listen port on Windows when WireGuard adapter holds the port On Windows the WireGuard-nt adapter from a previous run can keep the UDP port bound, causing GetFreePort to see it as busy and assign a random port. Additionally, concurrent wgctrl access and transient driver handle contention lead to ConfigureDevice failures. - Query the adapter's actual listen port before running GetFreePort so the port owned by our own adapter is not replaced - Add retry logic with backoff to apply() on Windows for transient 'file being used by another process' errors - Protect GetPeersFromDevice, GetPeer, and UpdatePeer with wgMutex to prevent concurrent device access - Add a delay between Close and Create in resetInterfaceFunc on Windows to let the kernel driver fully release the device handle --- functions/daemon.go | 23 +++++++++++---- functions/mqhandlers.go | 4 +++ functions/uninstall.go | 4 +++ wireguard/types.go | 2 ++ wireguard/wireguard.go | 63 +++++++++++++++++++++++++++++++++++++---- 5 files changed, 85 insertions(+), 11 deletions(-) diff --git a/functions/daemon.go b/functions/daemon.go index b6804900..5ecab489 100644 --- a/functions/daemon.go +++ b/functions/daemon.go @@ -184,12 +184,23 @@ func startGoRoutines(wg *sync.WaitGroup) context.CancelFunc { } if !netclientCfg.IsStaticPort { - if freeport, err := ncutils.GetFreePort(ncutils.NetclientDefaultPort, netclientCfg.ListenPort, false); err != nil { - slog.Warn("no free ports available for use by netclient", "error", err.Error()) - } else if freeport != netclientCfg.ListenPort { - slog.Info("port has changed", "old port", netclientCfg.ListenPort, "new port", freeport) - netclientCfg.ListenPort = freeport - updateConfig = true + // Check if the WireGuard adapter already owns the desired port before + // doing a UDP bind test. On Windows the adapter from a previous run may + // still be alive, and its port looks "busy" to net.ListenUDP even though + // it belongs to us. + devicePort, devErr := wireguard.GetDeviceListenPort() + portOwnedByDevice := devErr == nil && devicePort == netclientCfg.ListenPort && devicePort != 0 + + if !portOwnedByDevice { + if freeport, err := ncutils.GetFreePort(ncutils.NetclientDefaultPort, netclientCfg.ListenPort, false); err != nil { + slog.Warn("no free ports available for use by netclient", "error", err.Error()) + } else if freeport != netclientCfg.ListenPort { + slog.Info("port has changed", "old port", netclientCfg.ListenPort, "new port", freeport) + netclientCfg.ListenPort = freeport + updateConfig = true + } + } else { + slog.Info("listen port already held by wireguard adapter, keeping it", "port", devicePort) } if netclientCfg.WgPublicListenPort == 0 { diff --git a/functions/mqhandlers.go b/functions/mqhandlers.go index dc814c2f..c8b2188c 100644 --- a/functions/mqhandlers.go +++ b/functions/mqhandlers.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "reflect" + "runtime" "slices" "strconv" "strings" @@ -572,6 +573,9 @@ func resetInterfaceFunc() { defer mNMutex.Unlock() nc := wireguard.GetInterface() nc.Close() + if runtime.GOOS == "windows" { + time.Sleep(500 * time.Millisecond) + } nc = wireguard.NewNCIface(config.Netclient(), config.GetNodes()) nc.Create() if err := nc.Configure(); err != nil { diff --git a/functions/uninstall.go b/functions/uninstall.go index 68fa0224..52dbf534 100644 --- a/functions/uninstall.go +++ b/functions/uninstall.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "runtime" + "time" "github.com/gravitl/netclient/auth" "github.com/gravitl/netclient/config" @@ -75,6 +76,9 @@ func resetInterfaceUninstall(faults []error) []error { defer mNMutex.Unlock() nc := wireguard.GetInterface() nc.Close() + if runtime.GOOS == "windows" { + time.Sleep(500 * time.Millisecond) + } nc = wireguard.NewNCIface(config.Netclient(), config.GetNodes()) nc.Create() if err := nc.Configure(); err != nil { diff --git a/wireguard/types.go b/wireguard/types.go index 612e84ab..37e86516 100644 --- a/wireguard/types.go +++ b/wireguard/types.go @@ -277,6 +277,8 @@ func GetInterface() *NCIface { // NCIface.UpdatePeer - Updates Peers from provided PeerConfig func (n *NCIface) UpdatePeer(p wgtypes.PeerConfig) { + wgMutex.Lock() + defer wgMutex.Unlock() peers := []wgtypes.PeerConfig{} peers = append(peers, p) n.Config.ReplacePeers = false diff --git a/wireguard/wireguard.go b/wireguard/wireguard.go index 1d9ec1f8..700e7ec6 100644 --- a/wireguard/wireguard.go +++ b/wireguard/wireguard.go @@ -4,6 +4,9 @@ import ( "errors" "fmt" "net" + "runtime" + "strings" + "time" "github.com/gravitl/netclient/cache" "github.com/gravitl/netclient/config" @@ -97,13 +100,42 @@ func UpdatePeer(p *wgtypes.PeerConfig) error { func apply(c *wgtypes.Config) error { slog.Debug("applying wireguard config") - wg, err := wgctrl.New() - if err != nil { - return fmt.Errorf("wgctrl %w", err) + ifaceName := ncutils.GetInterfaceName() + + const maxRetries = 3 + for attempt := 0; attempt <= maxRetries; attempt++ { + wg, err := wgctrl.New() + if err != nil { + if runtime.GOOS == "windows" && attempt < maxRetries && isDeviceBusyError(err) { + slog.Warn("wgctrl busy, retrying", "attempt", attempt+1, "error", err) + time.Sleep(time.Duration(500*(attempt+1)) * time.Millisecond) + continue + } + return fmt.Errorf("wgctrl %w", err) + } + err = wg.ConfigureDevice(ifaceName, *c) + wg.Close() + if err == nil { + return nil + } + if runtime.GOOS == "windows" && attempt < maxRetries && isDeviceBusyError(err) { + slog.Warn("ConfigureDevice busy, retrying", "attempt", attempt+1, "error", err) + time.Sleep(time.Duration(500*(attempt+1)) * time.Millisecond) + continue + } + return err } - defer wg.Close() + return fmt.Errorf("ConfigureDevice failed after %d retries", maxRetries) +} - return wg.ConfigureDevice(ncutils.GetInterfaceName(), *c) +func isDeviceBusyError(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "being used by another process") || + strings.Contains(msg, "access is denied") || + strings.Contains(msg, "The process cannot access the file") } // returns if better endpoint has been calculated for this peer already @@ -140,6 +172,8 @@ func EndpointDetectedAlready(peerPubKey string) bool { } func GetPeersFromDevice(ifaceName string) (map[string]wgtypes.Peer, error) { + wgMutex.Lock() + defer wgMutex.Unlock() peerMap := make(map[string]wgtypes.Peer) wg, err := wgctrl.New() if err != nil { @@ -163,6 +197,8 @@ func GetPeersFromDevice(ifaceName string) (map[string]wgtypes.Peer, error) { // GetPeer - gets the peerinfo from the wg interface func GetPeer(ifaceName, peerPubKey string) (wgtypes.Peer, error) { + wgMutex.Lock() + defer wgMutex.Unlock() wg, err := wgctrl.New() if err != nil { return wgtypes.Peer{}, err @@ -185,6 +221,23 @@ func GetPeer(ifaceName, peerPubKey string) (wgtypes.Peer, error) { return wgtypes.Peer{}, fmt.Errorf("peer not found") } +// GetDeviceListenPort returns the current listen port from the WireGuard device. +// Returns 0 and an error if the device cannot be queried. +func GetDeviceListenPort() (int, error) { + wgMutex.Lock() + defer wgMutex.Unlock() + wg, err := wgctrl.New() + if err != nil { + return 0, err + } + defer wg.Close() + dev, err := wg.Device(ncutils.GetInterfaceName()) + if err != nil { + return 0, err + } + return dev.ListenPort, nil +} + // GetOriginalDefaulGw - fetches system's original default gw func GetOriginalDefaulGw() (gwIP net.IP, err error) { gwIP = config.Netclient().OriginalDefaultGatewayIp From 254d020ff2e51ba093590d16805e7864a3f625e1 Mon Sep 17 00:00:00 2001 From: abhishek9686 Date: Wed, 18 Mar 2026 17:32:48 +0530 Subject: [PATCH 2/2] NM-279: fix unreachable fallback error in apply() retry loop On the final retry attempt, busy errors were returned directly instead of falling through to the exhausted-retries message. Restructure so busy errors always continue the loop and the post-loop error with the retry count is the definitive exhaustion path. --- wireguard/wireguard.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/wireguard/wireguard.go b/wireguard/wireguard.go index 700e7ec6..6f5fb768 100644 --- a/wireguard/wireguard.go +++ b/wireguard/wireguard.go @@ -103,12 +103,16 @@ func apply(c *wgtypes.Config) error { ifaceName := ncutils.GetInterfaceName() const maxRetries = 3 + var lastErr error for attempt := 0; attempt <= maxRetries; attempt++ { wg, err := wgctrl.New() if err != nil { - if runtime.GOOS == "windows" && attempt < maxRetries && isDeviceBusyError(err) { - slog.Warn("wgctrl busy, retrying", "attempt", attempt+1, "error", err) - time.Sleep(time.Duration(500*(attempt+1)) * time.Millisecond) + if runtime.GOOS == "windows" && isDeviceBusyError(err) { + lastErr = err + if attempt < maxRetries { + slog.Warn("wgctrl busy, retrying", "attempt", attempt+1, "error", err) + time.Sleep(time.Duration(500*(attempt+1)) * time.Millisecond) + } continue } return fmt.Errorf("wgctrl %w", err) @@ -118,14 +122,17 @@ func apply(c *wgtypes.Config) error { if err == nil { return nil } - if runtime.GOOS == "windows" && attempt < maxRetries && isDeviceBusyError(err) { - slog.Warn("ConfigureDevice busy, retrying", "attempt", attempt+1, "error", err) - time.Sleep(time.Duration(500*(attempt+1)) * time.Millisecond) + if runtime.GOOS == "windows" && isDeviceBusyError(err) { + lastErr = err + if attempt < maxRetries { + slog.Warn("ConfigureDevice busy, retrying", "attempt", attempt+1, "error", err) + time.Sleep(time.Duration(500*(attempt+1)) * time.Millisecond) + } continue } return err } - return fmt.Errorf("ConfigureDevice failed after %d retries", maxRetries) + return fmt.Errorf("ConfigureDevice failed after %d retries: %w", maxRetries, lastErr) } func isDeviceBusyError(err error) bool {