fix(network): fix wifi page never scanning, never exiting, and UI freezes on connect

WiFiSetupPage never showed networks and could not be exited once entered.
Several compounding bugs were responsible across network.go and page.go.

Primary bugs fixed:

- WiFiSetupPage.Init() never assigned p.wifiDevice after FindDevices(),
  so startScan() always hit the nil guard and exited without launching
  a scan goroutine. No networks ever appeared in the list. A value copy
  of the Wi-Fi device is now taken from m.Devices immediately after
  FindDevices() returns, with an explicit error if no adapter is found.

- CheckInternetConnection() called FindDevices() on every tick, rebuilding
  m.Devices every 2 seconds. This caused [NET-030] log spam, invalidated
  any pointers held into the slice, and meant checkState()'s fresh Manager
  instances (which have an empty Devices slice) would never detect internet
  after Fix 2 was applied. CheckInternetConnection() now calls FindDevices()
  only when m.Devices is empty (fresh Manager), and does a cheaper per-device
  State property refresh when devices are already known (long-lived Manager).
  This satisfies both callers correctly.

- Connect() called WaitForInternet(30s, 5s) synchronously before returning,
  blocking the main UI thread for up to 35 seconds. It also returned an error
  if internet wasn't stable in time, even though AddAndActivateConnection had
  succeeded, causing the page to show "Failed to initiate connection" while NM
  was still negotiating in the background. WaitForInternet removed from
  Connect(); the page's existing checkConnectionStatusLoop and
  handleConnectionStatusUpdate already handle post-connection state tracking.

Additional bugs fixed:

- startScan() launched a new StartScan goroutine on every R-press without
  signalling the previous one to stop. Concurrent goroutines raced on
  resultsChan and accumulated indefinitely. stopScan is now closed and
  replaced before each scan, and checkConnectionStatusLoop is restarted
  with the new channel.

- Connect() always generated a new UUID and called AddAndActivateConnection
  without checking for an existing profile. Repeated connection attempts
  (e.g. wrong password then correct) accumulated duplicate stale profiles
  in NetworkManager. Existing profiles matching the SSID are now deleted
  via Settings.Connection.Delete before creating a new one.

- m.Devices was read and written concurrently from StartScan and
  CheckInternetConnection goroutines without synchronisation. A sync.RWMutex
  is added to Manager; state refreshes take a write lock, StartScan device
  lookup takes a read lock.

- Connect() checked wifiDevice.State using the caller's value copy taken at
  Init() time, which never updated. The current state is now read from
  m.Devices under lock at the point of connection.

- auth-alg: "open" was unconditionally set in the 802-11-wireless-security
  settings block. This field is only meaningful for WPA/WPA2-Personal and
  should be omitted for SAE (WPA3) and EAP variants. It is now only set
  when KeyMgmt is "wpa-psk".
This commit is contained in:
NeilClack
2026-06-24 22:41:48 -05:00
parent a7f551ccca
commit a4eff17b77
4 changed files with 114 additions and 13 deletions
+1
View File
@@ -97,6 +97,7 @@ func Run() {
os.Remove(testFile) os.Remove(testFile)
logger.AddFile(filepath.Join(dataDir, "log.txt")) logger.AddFile(filepath.Join(dataDir, "log.txt"))
logger.SetLevel = logger.LevelDebug
DB, err = db.Connect(filepath.Join(dataDir, "database.sqlite")) DB, err = db.Connect(filepath.Join(dataDir, "database.sqlite"))
if err != nil { if err != nil {
+86 -9
View File
@@ -8,6 +8,7 @@ import (
"net" "net"
"sort" "sort"
"strings" "strings"
"sync"
"time" "time"
"github.com/godbus/dbus/v5" "github.com/godbus/dbus/v5"
@@ -91,6 +92,7 @@ type Manager struct {
conn *dbus.Conn conn *dbus.Conn
nmObject dbus.BusObject nmObject dbus.BusObject
Devices []Device Devices []Device
mu sync.RWMutex // protects Devices
} }
// AccessPoint holds essential information about a discovered Wi-Fi network. // AccessPoint holds essential information about a discovered Wi-Fi network.
@@ -142,7 +144,7 @@ func (m *Manager) FindDevices() error {
var devicePaths []dbus.ObjectPath var devicePaths []dbus.ObjectPath
call := m.nmObject.Call(nmInterface+".GetDevices", 0) call := m.nmObject.Call(nmInterface+".GetDevices", 0)
if call.Err != nil { if call.Err != nil {
logger.Error("GetDevices D-Bus call failed: %w", call.Err) logger.Error("GetDevices D-Bus call failed: %v", call.Err)
return fmt.Errorf("GetDevices D-Bus call failed: %w", call.Err) return fmt.Errorf("GetDevices D-Bus call failed: %w", call.Err)
} }
if err := call.Store(&devicePaths); err != nil { if err := call.Store(&devicePaths); err != nil {
@@ -205,9 +207,34 @@ func (m *Manager) FindDevices() error {
// CheckInternetConnection performs a check for an active internet connection. // CheckInternetConnection performs a check for an active internet connection.
func (m *Manager) CheckInternetConnection() (*ConnectionStatus, error) { func (m *Manager) CheckInternetConnection() (*ConnectionStatus, error) {
logger.Debug("[NET-040] Checking internet connectivity") logger.Debug("[NET-040] Checking internet connectivity")
// Previously called FindDevices() here, which rebuilt m.Devices every 2s,
// causing [NET-030] log spam and invalidating any pointers held by callers.
// Device enumeration is done once in Init(); we only need fresh State values.
// If this is a fresh Manager with no devices yet (e.g. called from
// checkState which creates a new Manager each tick), do a full device
// discovery. If devices are already known (e.g. the WiFiSetupPage's
// long-lived Manager), only refresh State to avoid the [NET-030] spam
// and avoid rebuilding the slice which would invalidate held pointers.
if len(m.Devices) == 0 {
if err := m.FindDevices(); err != nil { if err := m.FindDevices(); err != nil {
return &ConnectionStatus{StatusError, "Failed to scan devices"}, err return &ConnectionStatus{StatusError, "Failed to scan devices"}, err
} }
} else {
m.mu.Lock()
for i := range m.Devices {
deviceObj := m.conn.Object(nmService, m.Devices[i].Path)
stateVariant, err := deviceObj.GetProperty(nmDeviceInterface + ".State")
if err != nil {
logger.Warning("[NET-041] Could not refresh state for device %s: %v", m.Devices[i].Path, err)
continue
}
if state, ok := stateVariant.Value().(uint32); ok {
m.Devices[i].State = state
}
}
m.mu.Unlock()
}
// Check Ethernet first (Priority) // Check Ethernet first (Priority)
for _, device := range m.Devices { for _, device := range m.Devices {
@@ -235,14 +262,22 @@ func (m *Manager) CheckInternetConnection() (*ConnectionStatus, error) {
// StartScan performs a Wi-Fi scan asynchronously. The optional stopChan can be used to abort. // StartScan performs a Wi-Fi scan asynchronously. The optional stopChan can be used to abort.
func (m *Manager) StartScan(resultsChan chan<- []AccessPoint, errorChan chan<- error, stopChan <-chan struct{}) { func (m *Manager) StartScan(resultsChan chan<- []AccessPoint, errorChan chan<- error, stopChan <-chan struct{}) {
logger.Info("[NET-050] Starting Wi‑Fi scan") logger.Info("[NET-050] Starting Wi‑Fi scan")
m.mu.RLock()
var wifiDevice *Device var wifiDevice *Device
for i := range m.Devices { for i := range m.Devices {
if m.Devices[i].Type == TypeWifi { if m.Devices[i].Type == TypeWifi {
wifiDevice = &m.Devices[i] wifiDevice = &m.Devices[i]
break break
} }
} }
m.mu.RUnlock()
if wifiDevice == nil { if wifiDevice == nil {
errorChan <- fmt.Errorf("no Wi-Fi device available for scanning") errorChan <- fmt.Errorf("no Wi-Fi device available for scanning")
return return
@@ -250,27 +285,35 @@ func (m *Manager) StartScan(resultsChan chan<- []AccessPoint, errorChan chan<- e
// Ensure Wi-Fi is enabled // Ensure Wi-Fi is enabled
nmObj := m.conn.Object(nmService, nmPath) nmObj := m.conn.Object(nmService, nmPath)
prop, err := nmObj.GetProperty(nmInterface + ".WirelessEnabled") prop, err := nmObj.GetProperty(nmInterface + ".WirelessEnabled")
if err != nil { if err != nil {
errorChan <- fmt.Errorf("could not read WirelessEnabled property: %w", err) errorChan <- fmt.Errorf("could not read WirelessEnabled property: %w", err)
return return
} }
if !prop.Value().(bool) { if !prop.Value().(bool) {
call := nmObj.Call("org.freedesktop.DBus.Properties.Set", 0, call := nmObj.Call("org.freedesktop.DBus.Properties.Set", 0,
nmInterface, nmInterface,
"WirelessEnabled", "WirelessEnabled",
dbus.MakeVariant(true), dbus.MakeVariant(true),
) )
if call.Err != nil { if call.Err != nil {
errorChan <- fmt.Errorf("could not enable Wi-Fi: %w", call.Err) errorChan <- fmt.Errorf("could not enable Wi-Fi: %w", call.Err)
return return
} }
time.Sleep(2 * time.Second) // Give NetworkManager time to bring up the device time.Sleep(2 * time.Second) // Give NetworkManager time to bring up the device
} }
// Request a scan // Request a scan
wifiDeviceWireless := m.conn.Object(nmService, wifiDevice.Path) wifiDeviceWireless := m.conn.Object(nmService, wifiDevice.Path)
call := wifiDeviceWireless.Call(nmWirelessDeviceInterface+".RequestScan", 0, make(map[string]dbus.Variant)) call := wifiDeviceWireless.Call(nmWirelessDeviceInterface+".RequestScan", 0, make(map[string]dbus.Variant))
if call.Err != nil { if call.Err != nil {
errorChan <- fmt.Errorf("scan request failed: %w", call.Err) errorChan <- fmt.Errorf("scan request failed: %w", call.Err)
return return
@@ -284,11 +327,14 @@ func (m *Manager) StartScan(resultsChan chan<- []AccessPoint, errorChan chan<- e
} }
var apPaths []dbus.ObjectPath var apPaths []dbus.ObjectPath
call = wifiDeviceWireless.Call(nmWirelessDeviceInterface+".GetAllAccessPoints", 0) call = wifiDeviceWireless.Call(nmWirelessDeviceInterface+".GetAllAccessPoints", 0)
if call.Err != nil { if call.Err != nil {
errorChan <- fmt.Errorf("could not retrieve network list: %w", call.Err) errorChan <- fmt.Errorf("could not retrieve network list: %w", call.Err)
return return
} }
if err := call.Store(&apPaths); err != nil { if err := call.Store(&apPaths); err != nil {
errorChan <- fmt.Errorf("could not parse network list: %w", err) errorChan <- fmt.Errorf("could not parse network list: %w", err)
return return
@@ -299,6 +345,7 @@ func (m *Manager) StartScan(resultsChan chan<- []AccessPoint, errorChan chan<- e
errorChan <- err errorChan <- err
return return
} }
resultsChan <- processedAPs resultsChan <- processedAPs
} }
@@ -386,11 +433,40 @@ func (m *Manager) Connect(wifiDevice Device, ap AccessPoint, passphrase string)
if wifiDevice.Type != TypeWifi { if wifiDevice.Type != TypeWifi {
return fmt.Errorf("cannot connect: provided device is not a Wi-Fi device") return fmt.Errorf("cannot connect: provided device is not a Wi-Fi device")
} }
m.mu.RLock()
currentState := wifiDevice.State
for _, d := range m.Devices {
if d.Path == wifiDevice.Path {
currentState = d.State
break
}
}
m.mu.RUnlock()
if wifiDevice.State == nmDeviceStateFailed || if currentState == nmDeviceStateFailed ||
wifiDevice.State == nmDeviceStateUnmanaged || currentState == nmDeviceStateUnmanaged ||
wifiDevice.State == nmDeviceStateUnavailable { currentState == nmDeviceStateUnavailable {
return fmt.Errorf("cannot connect: device is in an error state (state=%d)", wifiDevice.State) return fmt.Errorf("cannot connect: device is in an error state (state=%d)", currentState)
}
// Delete any existing NM connection profile for this SSID to avoid
// accumulating duplicates across connection attempts.
settingsObj := m.conn.Object(nmService, "/org/freedesktop/NetworkManager/Settings")
var existingPaths []dbus.ObjectPath
if call := settingsObj.Call("org.freedesktop.NetworkManager.Settings.ListConnections", 0); call.Err == nil {
_ = call.Store(&existingPaths)
for _, cp := range existingPaths {
cObj := m.conn.Object(nmService, cp)
var csettings map[string]map[string]dbus.Variant
if c2 := cObj.Call("org.freedesktop.NetworkManager.Settings.Connection.GetSettings", 0); c2.Err == nil {
if err := c2.Store(&csettings); err == nil {
if id, ok := csettings["connection"]["id"].Value().(string); ok && id == ap.SSID {
logger.Info("[NET-073] Deleting existing profile for %q before reconnect", ap.SSID)
_ = cObj.Call("org.freedesktop.NetworkManager.Settings.Connection.Delete", 0)
}
}
}
}
} }
settings := map[string]map[string]dbus.Variant{ settings := map[string]map[string]dbus.Variant{
@@ -435,10 +511,11 @@ func (m *Manager) Connect(wifiDevice Device, ap AccessPoint, passphrase string)
logger.Info("[NET-072] Connection created successfully. Connection=%s Active=%s", connectionPath, activeConnectionPath) logger.Info("[NET-072] Connection created successfully. Connection=%s Active=%s", connectionPath, activeConnectionPath)
// Wait for the connection to be fully established // Removed WaitForInternet() call here. It blocked the caller's thread
if _, err := m.WaitForInternet(30*time.Second, 5*time.Second); err != nil { // for up to 35s and returned an error even on successful NM initiation
return fmt.Errorf("failed to establish internet connection: %w", err) // if internet wasn't stable in time. The page's checkConnectionStatusLoop
} // and handleConnectionStatusUpdate handle post-connection state tracking.
logger.Info("[NET-072] Connection initiated. Connection=%s Active=%s", connectionPath, activeConnectionPath)
return nil return nil
} }
+22 -2
View File
@@ -97,8 +97,23 @@ func (p *WiFiSetupPage) Init(ctx *PageCtx) error {
return fmt.Errorf("failed to find network devices: %w. Try rebooting the computer. If this is your 3rd reboot, call for help", err) return fmt.Errorf("failed to find network devices: %w. Try rebooting the computer. If this is your 3rd reboot, call for help", err)
} }
// FindDevices() rebuilds m.Devices each call, so store a VALUE copy,
// not a pointer into the slice — it would be invalidated on next call.
for i := range p.networkManager.Devices {
if p.networkManager.Devices[i].Type == network.TypeWifi {
device := p.networkManager.Devices[i]
p.wifiDevice = &device
break
}
}
if p.wifiDevice == nil {
p.networkManager.Close()
p.networkManager = nil
return fmt.Errorf("no Wi-Fi adapter found. Try rebooting the computer. If this is your 3rd reboot, call for help")
}
// Create UI elements // Create UI elements
white := sdl.Color{R: 255, G: 255, B: 255, A: 255} white := colors.White
p.titleItem, err = NewStringItem("Wi-Fi Setup", p.applicationFont, white) p.titleItem, err = NewStringItem("Wi-Fi Setup", p.applicationFont, white)
if err != nil { if err != nil {
return fmt.Errorf("failed to create title item: %w", err) return fmt.Errorf("failed to create title item: %w", err)
@@ -117,7 +132,6 @@ func (p *WiFiSetupPage) Init(ctx *PageCtx) error {
// Start scanning and status monitoring // Start scanning and status monitoring
p.startScan() p.startScan()
go p.checkConnectionStatusLoop()
return nil return nil
} }
@@ -143,6 +157,11 @@ func (p *WiFiSetupPage) checkConnectionStatusLoop() {
// startScan triggers a new asynchronous network scan. // startScan triggers a new asynchronous network scan.
func (p *WiFiSetupPage) startScan() { func (p *WiFiSetupPage) startScan() {
if p.stopScan != nil {
close(p.stopScan)
}
p.stopScan = make(chan struct{})
p.isScanning = true p.isScanning = true
p.selectedIndex = 0 p.selectedIndex = 0
p.scrollOffset = 0 p.scrollOffset = 0
@@ -150,6 +169,7 @@ func (p *WiFiSetupPage) startScan() {
if p.wifiDevice != nil { if p.wifiDevice != nil {
go p.networkManager.StartScan(p.resultsChan, p.errorChan, p.stopScan) go p.networkManager.StartScan(p.resultsChan, p.errorChan, p.stopScan)
go p.checkConnectionStatusLoop()
} else { } else {
p.isScanning = false p.isScanning = false
} }
+3
View File
@@ -0,0 +1,3 @@
* Networking Issues
- The networking package needs a LOT more logging... Nothing gets output when things crash.
-