From 61da51ed2e66450f0a481afb9bce782c68ad63de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Mon, 1 Jun 2026 21:11:32 +0200 Subject: [PATCH] client/peer: don't fan out unchanged management/signal state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MarkManagement{Connected,Disconnected} and MarkSignal{Connected, Disconnected} fired notifyStateChange unconditionally. The connect goroutine re-marks the same state on every health-check cycle, so a steady "connected -> connected" re-mark pushed a full SubscribeStatus snapshot to every consumer each time — flooding the desktop UI (and its tray) with identical Connected snapshots. Guard each with an early return when neither the state nor the error actually changed, so only real transitions wake SubscribeStatus subscribers. The notifier already deduplicates, so collapsing both calls under one guard is safe. --- client/internal/peer/status.go | 19 +++++++++++++++ client/internal/peer/status_test.go | 36 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/client/internal/peer/status.go b/client/internal/peer/status.go index 64722d593..c09124695 100644 --- a/client/internal/peer/status.go +++ b/client/internal/peer/status.go @@ -882,6 +882,13 @@ func (d *Status) CleanLocalPeerState() { // MarkManagementDisconnected sets ManagementState to disconnected func (d *Status) MarkManagementDisconnected(err error) { d.mux.Lock() + // Health checks re-mark the same state on every probe; skip the fan-out + // when nothing actually changed so we don't flood SubscribeStatus + // consumers with identical snapshots. + if !d.managementState && errors.Is(d.managementError, err) { + d.mux.Unlock() + return + } d.managementState = false d.managementError = err mgm := d.managementState @@ -895,6 +902,10 @@ func (d *Status) MarkManagementDisconnected(err error) { // MarkManagementConnected sets ManagementState to connected func (d *Status) MarkManagementConnected() { d.mux.Lock() + if d.managementState && d.managementError == nil { + d.mux.Unlock() + return + } d.managementState = true d.managementError = nil mgm := d.managementState @@ -936,6 +947,10 @@ func (d *Status) UpdateLazyConnection(enabled bool) { // MarkSignalDisconnected sets SignalState to disconnected func (d *Status) MarkSignalDisconnected(err error) { d.mux.Lock() + if !d.signalState && errors.Is(d.signalError, err) { + d.mux.Unlock() + return + } d.signalState = false d.signalError = err mgm := d.managementState @@ -949,6 +964,10 @@ func (d *Status) MarkSignalDisconnected(err error) { // MarkSignalConnected sets SignalState to connected func (d *Status) MarkSignalConnected() { d.mux.Lock() + if d.signalState && d.signalError == nil { + d.mux.Unlock() + return + } d.signalState = true d.signalError = nil mgm := d.managementState diff --git a/client/internal/peer/status_test.go b/client/internal/peer/status_test.go index 8d889b0ae..9dbc6af08 100644 --- a/client/internal/peer/status_test.go +++ b/client/internal/peer/status_test.go @@ -275,3 +275,39 @@ func TestGetFullStatus(t *testing.T) { assert.Equal(t, signalState, fullStatus.SignalState, "signal status should be equal") assert.ElementsMatch(t, []State{peerState1, peerState2}, fullStatus.Peers, "peers states should match") } + +// notified reports whether a state-change tick is pending on ch, draining it. +func notified(ch <-chan struct{}) bool { + select { + case <-ch: + return true + default: + return false + } +} + +func TestMarkServerStateDoesNotNotifyWhenUnchanged(t *testing.T) { + status := NewRecorder("https://mgm") + _, ch := status.SubscribeToStateChanges() + + // First transition is a real change and must notify. + status.MarkManagementConnected() + require.True(t, notified(ch), "first connect should notify") + + // Re-marking the same state must not notify again. + status.MarkManagementConnected() + assert.False(t, notified(ch), "redundant connect should not notify") + + // Same for signal. + status.MarkSignalConnected() + require.True(t, notified(ch), "first signal connect should notify") + status.MarkSignalConnected() + assert.False(t, notified(ch), "redundant signal connect should not notify") + + // A genuine change (disconnect with an error) notifies again. + err := errors.New("boom") + status.MarkManagementDisconnected(err) + require.True(t, notified(ch), "disconnect should notify") + status.MarkManagementDisconnected(err) + assert.False(t, notified(ch), "redundant disconnect should not notify") +}