diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index b5c380d7c..b30084293 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -462,7 +462,8 @@ func TestGetStateRefreshesLiveAlertSnapshots(t *testing.T) { source := string(data) for _, snippet := range []string{ - "state := m.state.GetSnapshot()", + "currentState := m.state", + "state := currentState.GetSnapshot()", "state.ActiveAlerts = m.activeAlertsSnapshot()", "state.RecentlyResolved = m.recentlyResolvedAlertsSnapshot()", } { diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 486079b4d..b37b64020 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -3163,16 +3163,26 @@ func (m *Monitor) GetState() models.StateSnapshot { } return state } - if m.state == nil { + // Mock-mode transitions replace the state under m.mu. Capture the fully + // initialized pointer under the same lock, then let State protect its own + // snapshot without holding the monitor lock across downstream readers. + m.mu.RLock() + currentState := m.state + m.mu.RUnlock() + if currentState == nil { return models.StateSnapshot{} } - state := m.state.GetSnapshot() + state := currentState.GetSnapshot() // Keep externally served alert arrays aligned with the live alert manager // even between explicit sync points, so APIs do not expose stale alert // counts or recently resolved incidents from cached state. - state.ActiveAlerts = m.activeAlertsSnapshot() - state.RecentlyResolved = m.recentlyResolvedAlertsSnapshot() + if m.alertManager != nil { + state.ActiveAlerts = m.activeAlertsSnapshot() + state.RecentlyResolved = m.recentlyResolvedAlertsSnapshot() + } + // Without an alert manager, retain alerts from this same captured state; + // the fallback helpers would read the replaceable pointer again. // Surface filesystems reported by a unified pulse-agent inside a guest // (for example ZFS mounts that qemu-guest-agent's get-fsinfo cannot see // on PBS, #1438) in the guest overview disk listing. diff --git a/internal/monitoring/monitor_state_reset_test.go b/internal/monitoring/monitor_state_reset_test.go new file mode 100644 index 000000000..46fa8399f --- /dev/null +++ b/internal/monitoring/monitor_state_reset_test.go @@ -0,0 +1,82 @@ +package monitoring + +import ( + "sync" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/mock" + "github.com/rcourtman/pulse-go-rewrite/internal/models" +) + +// Exercise the production reset boundary without Start or its background +// goroutines: shutting down a test differently cannot repair this race. +func TestGetStateConcurrentReset(t *testing.T) { + previous := mock.IsMockEnabled() + mustSetMockEnabled(t, false) + defer mustSetMockEnabled(t, previous) + + startTime := time.Unix(1700000000, 0) + m := &Monitor{startTime: startTime} + m.mu.Lock() + m.resetStateLocked() + m.mu.Unlock() + + start := make(chan struct{}) + var workers sync.WaitGroup + workers.Add(2) + go func() { + defer workers.Done() + <-start + for i := 0; i < 1000; i++ { + m.mu.Lock() + m.resetStateLocked() + m.mu.Unlock() + } + }() + go func() { + defer workers.Done() + <-start + for i := 0; i < 1000; i++ { + snapshot := m.GetState() + if snapshot.Stats.StartTime != startTime || snapshot.Stats.Version != "2.0.0-go" { + t.Errorf("reader observed partially initialized reset stats: %+v", snapshot.Stats) + return + } + } + }() + close(start) + workers.Wait() +} + +func TestGetStateResetSnapshotIsolation(t *testing.T) { + previous := mock.IsMockEnabled() + mustSetMockEnabled(t, false) + defer mustSetMockEnabled(t, previous) + + m := &Monitor{state: models.NewState(), startTime: time.Unix(1700000000, 0)} + m.state.UpdateNodes([]models.Node{{ID: "old-node", Name: "old-node"}}) + m.state.UpdateActiveAlerts([]models.Alert{{ID: "old-alert"}}) + m.state.UpdateRecentlyResolved([]models.ResolvedAlert{{Alert: models.Alert{ID: "old-resolved"}}}) + before := m.GetState() + m.mu.Lock() + m.resetStateLocked() + m.mu.Unlock() + after := m.GetState() + if len(before.Nodes) != 1 || before.Nodes[0].ID != "old-node" { + t.Fatal("reset changed the previously returned snapshot") + } + if len(before.ActiveAlerts) != 1 || before.ActiveAlerts[0].ID != "old-alert" || + len(before.RecentlyResolved) != 1 || before.RecentlyResolved[0].ID != "old-resolved" { + t.Fatal("snapshot lost state-backed alerts without an alert manager") + } + if len(after.ActiveAlerts) != 0 || len(after.RecentlyResolved) != 0 { + t.Fatal("reset retained stale state-backed alerts") + } + if len(after.Nodes) != 0 || after.Stats.StartTime != m.startTime || after.Stats.Version != "2.0.0-go" { + t.Fatalf("reset did not expose an initialized empty state: %+v", after) + } + if snapshot := (&Monitor{}).GetState(); len(snapshot.Nodes) != 0 { + t.Fatal("nil state must return an empty snapshot") + } +}