Merge branch 'fix/tray-menu-item-leak' into test/gui-memory-leak-fix

client/ui/tray.go

@@ -67,9 +67,8 @@ type Tray struct {
 	loc       *Localizer
 
 	// menu and the *Item/*Submenu fields below are reassigned by buildMenu
-	// on every relayout — touch them only with menuMu held. Exceptions:
+	// on every relayout, which destroys the replaced tree — locate items and
-	// the Connect/Disconnect OnClick closures capture their own item, and
+	// call them with menuMu held, so a relayout can't destroy one mid-call.
-	// refreshSessionExpiresLabel snapshots its item under menuMu.
 	menu       *application.Menu
 	statusItem *application.MenuItem
 	// sessionExpiresItem shows the SSO deadline as a remaining-time label,
@@ -286,6 +285,7 @@ func (t *Tray) relayoutMenu() {
 	t.menuMu.Lock()
 	defer t.menuMu.Unlock()
 
+	old := t.menu
 	t.menu = t.buildMenu()
 
 	t.statusMu.Lock()
@@ -356,6 +356,10 @@ func (t *Tray) relayoutMenu() {
 	// Single push of the whole tree: on Linux one LayoutUpdated with fresh
 	// container ids; on darwin an NSMenu rebuild against the cached pointer.
 	t.tray.SetMenu(t.menu)
+
+	if old != nil {
+		old.Destroy()
+	}
 }
 
 func (t *Tray) buildMenu() *application.Menu {
@@ -371,13 +375,11 @@ func (t *Tray) buildMenu() *application.Menu {
 
 	menu.AddSeparator()
 
-	// The OnClick closures capture the local item because t.upItem/t.downItem
-	// are menuMu-guarded and must not be read from the click goroutine.
 	upItem := menu.Add(t.loc.T("tray.menu.connect"))
-	upItem.OnClick(func(*application.Context) { t.handleConnect(upItem) })
+	upItem.OnClick(func(*application.Context) { t.handleConnect() })
 	t.upItem = upItem
 	downItem := menu.Add(t.loc.T("tray.menu.disconnect"))
-	downItem.OnClick(func(*application.Context) { t.handleDisconnect(downItem) })
+	downItem.OnClick(func(*application.Context) { t.handleDisconnect() })
 	downItem.SetHidden(true)
 	t.downItem = downItem
 
@@ -469,9 +471,7 @@ func (t *Tray) handleQuit() {
 	t.app.Quit()
 }
 
-// handleConnect receives the clicked item from the buildMenu closure —
+func (t *Tray) handleConnect() {
-// t.upItem is menuMu-guarded and must not be read here.
-func (t *Tray) handleConnect(upItem *application.MenuItem) {
 	// NeedsLogin/SessionExpired/LoginFailed won't honor a plain Up RPC — they
 	// need the Login → WaitSSOLogin → Up sequence. Emit EventTriggerLogin so
 	// the React startLogin() (which owns the BrowserLogin popup) drives it;
@@ -485,7 +485,7 @@ func (t *Tray) handleConnect(upItem *application.MenuItem) {
 		t.app.Event.Emit(services.EventTriggerLogin)
 		return
 	}
-	upItem.SetEnabled(false)
+	t.setItemEnabled(func() *application.MenuItem { return t.upItem }, false)
 	// Arm the SSO auto-handoff: Up() is async and the daemon may flip to
 	// NeedsLogin on an SSO peer with no cached token. applyStatus consumes the
 	// flag on that transition to trigger browser-login without a second Connect
@@ -500,18 +500,25 @@ func (t *Tray) handleConnect(upItem *application.MenuItem) {
 			t.statusMu.Lock()
 			t.pendingConnectLogin = false
 			t.statusMu.Unlock()
-			upItem.SetEnabled(true)
+			t.setItemEnabled(func() *application.MenuItem { return t.upItem }, true)
 		}
 	}()
 }
 
+func (t *Tray) setItemEnabled(get func() *application.MenuItem, enabled bool) {
+	t.menuMu.Lock()
+	defer t.menuMu.Unlock()
+	if item := get(); item != nil {
+		item.SetEnabled(enabled)
+	}
+}
+
 // handleDisconnect aborts any in-flight profile switch before sending Down —
 // otherwise the switcher's queued Up would reconnect right after, making the
 // click a no-op. Also clears Peers' optimistic-Connecting guard so the daemon's
 // Idle push paints through instead of being swallowed by the suppression filter.
-// Receives the clicked item from the buildMenu closure (see handleConnect).
+func (t *Tray) handleDisconnect() {
-func (t *Tray) handleDisconnect(downItem *application.MenuItem) {
+	t.setItemEnabled(func() *application.MenuItem { return t.downItem }, false)
-	downItem.SetEnabled(false)
 	t.profileMu.Lock()
 	if t.switchCancel != nil {
 		t.switchCancel()
@@ -523,7 +530,7 @@ func (t *Tray) handleDisconnect(downItem *application.MenuItem) {
 		if err := t.svc.Connection.Down(context.Background()); err != nil {
 			log.Errorf("disconnect: %v", err)
 			t.notifyError(t.loc.T("notify.error.disconnect"))
-			downItem.SetEnabled(true)
+			t.setItemEnabled(func() *application.MenuItem { return t.downItem }, true)
 		}
 	}()
 }

client/ui/tray_profiles.go

@@ -5,6 +5,7 @@ package main
 import (
 	"context"
 	"fmt"
+	"slices"
 	"sort"
 
 	log "github.com/sirupsen/logrus"
@@ -59,10 +60,11 @@ func (t *Tray) loadConfig() {
 	t.profileMu.Unlock()
 }
 
-// loadProfiles fetches the profile list and relayouts the menu. Also called
+// loadProfiles fetches the profile list and relayouts the menu when the rows
-// from applyStatus to catch flips from another channel (CLI, autoconnect),
+// changed. Also called from applyStatus to catch flips from another channel
-// since the daemon emits no active-profile event. Full relayout (not
+// (CLI, autoconnect), since the daemon emits no active-profile event. Full
-// Clear()+Add()) is required for KDE/Plasma — see relayoutMenu's doc comment.
+// relayout (not Clear()+Add()) is required for KDE/Plasma — see relayoutMenu's
+// doc comment.
 func (t *Tray) loadProfiles() {
 	t.profileLoadMu.Lock()
 	defer t.profileLoadMu.Unlock()
@@ -80,11 +82,14 @@ func (t *Tray) loadProfiles() {
 	}
 
 	t.profilesMu.Lock()
+	changed := username != t.profilesUser || !slices.Equal(profiles, t.profiles)
 	t.profiles = profiles
 	t.profilesUser = username
 	t.profilesMu.Unlock()
 
-	t.relayoutMenu()
+	if changed {
+		t.relayoutMenu()
+	}
 }
 
 // fillProfileSubmenu paints cached profile rows into the freshly built submenu.

client/ui/tray_session.go

@@ -104,21 +104,19 @@ func sessionRefreshInterval(remaining time.Duration) time.Duration {
 }
 
 // refreshSessionExpiresLabel updates only the countdown label, no relayout, to avoid disturbing an open menu.
-// The item is snapshotted under menuMu since buildMenu reassigns it on every relayout.
+// menuMu is held across the SetLabel so a relayout can't destroy the item mid-call.
 func (t *Tray) refreshSessionExpiresLabel() {
-	t.menuMu.Lock()
-	item := t.sessionExpiresItem
-	t.menuMu.Unlock()
-	if item == nil {
-		return
-	}
 	t.sessionMu.Lock()
 	deadline := t.sessionExpiresAt
 	t.sessionMu.Unlock()
 	if deadline.IsZero() {
 		return
 	}
-	item.SetLabel(t.sessionRowLabel(deadline))
+	t.menuMu.Lock()
+	defer t.menuMu.Unlock()
+	if t.sessionExpiresItem != nil {
+		t.sessionExpiresItem.SetLabel(t.sessionRowLabel(deadline))
+	}
 }
 
 func (t *Tray) sessionRowLabel(deadline time.Time) string {