diff --git a/client/ui/tray.go b/client/ui/tray.go index 3050d159acd..7a1f7477059 100644 --- a/client/ui/tray.go +++ b/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: - // the Connect/Disconnect OnClick closures capture their own item, and - // refreshSessionExpiresLabel snapshots its item under menuMu. + // on every relayout, which destroys the replaced tree — locate items and + // call them with menuMu held, so a relayout can't destroy one mid-call. 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 — -// t.upItem is menuMu-guarded and must not be read here. -func (t *Tray) handleConnect(upItem *application.MenuItem) { +func (t *Tray) handleConnect() { // 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(downItem *application.MenuItem) { - downItem.SetEnabled(false) +func (t *Tray) handleDisconnect() { + t.setItemEnabled(func() *application.MenuItem { return t.downItem }, 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) } }() } diff --git a/client/ui/tray_profiles.go b/client/ui/tray_profiles.go index 5251c138efd..cf0ed87a917 100644 --- a/client/ui/tray_profiles.go +++ b/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 -// from applyStatus to catch flips from another channel (CLI, autoconnect), -// since the daemon emits no active-profile event. Full relayout (not -// Clear()+Add()) is required for KDE/Plasma — see relayoutMenu's doc comment. +// loadProfiles fetches the profile list and relayouts the menu when the rows +// changed. Also called from applyStatus to catch flips from another channel +// (CLI, autoconnect), since the daemon emits no active-profile event. Full +// 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. diff --git a/client/ui/tray_session.go b/client/ui/tray_session.go index 885fdb348fd..45caf2a37fc 100644 --- a/client/ui/tray_session.go +++ b/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 {