[client] Fix tray menu item leak on relayout

This commit is contained in:
Zoltán Papp
2026-07-28 14:53:15 +02:00
parent 8a43f4f943
commit 29b97340d6
2 changed files with 34 additions and 21 deletions

View File

@@ -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) — touch them only
// with menuMu held; snapshot under the lock, then call the item.
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,26 @@ 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()
item := get()
t.menuMu.Unlock()
if 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 +531,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)
}
}()
}

View File

@@ -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.