From 04d499b1fdb749216ac796c7103d7ba8121fd841 Mon Sep 17 00:00:00 2001 From: Tobsec <8700196+tobsec@users.noreply.github.com> Date: Wed, 25 Mar 2026 13:26:53 +0100 Subject: [PATCH 1/2] client/ui: fix Exit Node submenu separator accumulation on Windows On Windows the tray uses a background poller (every 10s) instead of TrayOpenedCh to keep the Exit Node menu fresh. Each poll that has a selected exit node called s.mExitNode.AddSeparator() before the "Deselect All" item. Because AddSeparator() returns no handle the separator was never removed in the cleanup pass of recreateExitNodeMenu(), while every other item (exit node checkboxes and the "Deselect All" entry) was properly tracked and removed. After the client has been running for a while with an exit node selected this leaves hundreds of separator lines stacked in the submenu, filling the screen height with blank entries (#4702). On Linux/FreeBSD this is masked because the parent mExitNode item itself is removed and recreated each cycle, wiping all children including orphaned separators. Fix: replace the untracked AddSeparator() call with a regular disabled sub-menu item that is stored in mExitNodeSeparator and removed at the start of each recreateExitNodeMenu() call alongside mExitNodeDeselectAll. Fixes #4702 --- client/ui/client_ui.go | 1 + client/ui/network.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/client/ui/client_ui.go b/client/ui/client_ui.go index 0574e53d0be..b1e0aec418c 100644 --- a/client/ui/client_ui.go +++ b/client/ui/client_ui.go @@ -324,6 +324,7 @@ type serviceClient struct { exitNodeMu sync.Mutex mExitNodeItems []menuHandler exitNodeRetryCancel context.CancelFunc + mExitNodeSeparator *systray.MenuItem mExitNodeDeselectAll *systray.MenuItem logFile string wLoginURL fyne.Window diff --git a/client/ui/network.go b/client/ui/network.go index ed03f5adadd..c2f31ba2596 100644 --- a/client/ui/network.go +++ b/client/ui/network.go @@ -421,6 +421,10 @@ func (s *serviceClient) recreateExitNodeMenu(exitNodes []*proto.Network) { node.Remove() } s.mExitNodeItems = nil + if s.mExitNodeSeparator != nil { + s.mExitNodeSeparator.Remove() + s.mExitNodeSeparator = nil + } if s.mExitNodeDeselectAll != nil { s.mExitNodeDeselectAll.Remove() s.mExitNodeDeselectAll = nil @@ -453,7 +457,9 @@ func (s *serviceClient) recreateExitNodeMenu(exitNodes []*proto.Network) { } if showDeselectAll { - s.mExitNode.AddSeparator() + sep := s.mExitNode.AddSubMenuItem("───────────────", "") + sep.Disable() + s.mExitNodeSeparator = sep deselectAllItem := s.mExitNode.AddSubMenuItem("Deselect All", "Deselect All") s.mExitNodeDeselectAll = deselectAllItem go func() { From 8e001998cd90289440617a57944cc0bc84c89633 Mon Sep 17 00:00:00 2001 From: Tobsec <8700196+tobsec@users.noreply.github.com> Date: Sat, 28 Mar 2026 13:36:01 +0100 Subject: [PATCH 2/2] client/ui: extract addExitNodeDeselectAll to reduce cognitive complexity Move the separator + deselect-all creation and its goroutine listener out of recreateExitNodeMenu into a dedicated helper, bringing the function's cognitive complexity back under the SonarCloud threshold. --- client/ui/network.go | 50 ++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/client/ui/network.go b/client/ui/network.go index c2f31ba2596..571e871bbf2 100644 --- a/client/ui/network.go +++ b/client/ui/network.go @@ -457,33 +457,37 @@ func (s *serviceClient) recreateExitNodeMenu(exitNodes []*proto.Network) { } if showDeselectAll { - sep := s.mExitNode.AddSubMenuItem("───────────────", "") - sep.Disable() - s.mExitNodeSeparator = sep - deselectAllItem := s.mExitNode.AddSubMenuItem("Deselect All", "Deselect All") - s.mExitNodeDeselectAll = deselectAllItem - go func() { - for { - _, ok := <-deselectAllItem.ClickedCh - if !ok { - // channel closed: exit the goroutine - return - } - exitNodes, err := s.handleExitNodeMenuDeselectAll() - if err != nil { - log.Warnf("failed to handle deselect all exit nodes: %v", err) - } else { - s.exitNodeMu.Lock() - s.recreateExitNodeMenu(exitNodes) - s.exitNodeMu.Unlock() - } - } - - }() + s.addExitNodeDeselectAll() } } +func (s *serviceClient) addExitNodeDeselectAll() { + sep := s.mExitNode.AddSubMenuItem("───────────────", "") + sep.Disable() + s.mExitNodeSeparator = sep + + deselectAllItem := s.mExitNode.AddSubMenuItem("Deselect All", "Deselect All") + s.mExitNodeDeselectAll = deselectAllItem + + go func() { + for { + _, ok := <-deselectAllItem.ClickedCh + if !ok { + return + } + exitNodes, err := s.handleExitNodeMenuDeselectAll() + if err != nil { + log.Warnf("failed to handle deselect all exit nodes: %v", err) + } else { + s.exitNodeMu.Lock() + s.recreateExitNodeMenu(exitNodes) + s.exitNodeMu.Unlock() + } + } + }() +} + func (s *serviceClient) getExitNodes(conn proto.DaemonServiceClient) ([]*proto.Network, error) { ctx, cancel := context.WithTimeout(s.ctx, defaultFailTimeout) defer cancel()