From 0f0dfbe8b5eb0b7630a0af39772300e2c775274d Mon Sep 17 00:00:00 2001 From: Jake Wang Date: Mon, 7 Sep 2026 15:37:38 -0400 Subject: [PATCH] fix: prevent monitor health-check shutdown deadlock --- VERSION | 2 +- docs/RELEASE_2_1.md | 14 ++++- internal/wal/monitor.go | 16 ++++- internal/wal/monitor_test.go | 119 +++++++++++++++++++++++++++++++++++ npm/package.json | 2 +- server.json | 4 +- 6 files changed, 147 insertions(+), 10 deletions(-) create mode 100644 internal/wal/monitor_test.go diff --git a/VERSION b/VERSION index 7ec1d6d..3e3c2f1 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.1.0 +2.1.1 diff --git a/docs/RELEASE_2_1.md b/docs/RELEASE_2_1.md index 7f81439..845bb9a 100644 --- a/docs/RELEASE_2_1.md +++ b/docs/RELEASE_2_1.md @@ -1,8 +1,16 @@ # Argon 2.1: correctness and workflow hardening -This release candidate addresses the September 2026 project review. The version -in `VERSION`, CLI, API, npm metadata and MCP manifest is 2.1.0. A prepared version -does not mean that a GitHub release, npm package or hosted deployment is live. +Argon 2.1.0 was published on September 7, 2026 to address the project review. +Its [GitHub release](https://github.com/argon-lab/argon/releases/tag/v2.1.0), npm +package and MCP registry entry are available. Hosted deployment status is tracked +separately from package publication. + +The 2.1.1 patch fixes a monitor mutex deadlock discovered during post-release +shutdown verification. After the first health-check tick, alert resolution could +try to acquire the mutex already held by the health loop; `argon console` then +waited indefinitely in `Monitor.Stop`. Internal alert helpers now reuse the held +lock. Regressions cover actual ticker shutdown, successful checks, repeated +failures and alert recovery. The 2.1.0 tag and assets remain unchanged. ## Changes diff --git a/internal/wal/monitor.go b/internal/wal/monitor.go index 01766ee..f43a020 100644 --- a/internal/wal/monitor.go +++ b/internal/wal/monitor.go @@ -194,6 +194,11 @@ func (m *Monitor) AddHealthCheck(check HealthCheck) { func (m *Monitor) TriggerAlert(level AlertLevel, title, message string, data map[string]interface{}) { m.mu.Lock() defer m.mu.Unlock() + m.triggerAlertLocked(level, title, message, data) +} + +// triggerAlertLocked requires m.mu to be held by the caller. +func (m *Monitor) triggerAlertLocked(level AlertLevel, title, message string, data map[string]interface{}) { alert := Alert{ Level: level, @@ -215,6 +220,11 @@ func (m *Monitor) TriggerAlert(level AlertLevel, title, message string, data map func (m *Monitor) ResolveAlert(title string) { m.mu.Lock() defer m.mu.Unlock() + m.resolveAlertLocked(title) +} + +// resolveAlertLocked requires m.mu to be held by the caller. +func (m *Monitor) resolveAlertLocked(title string) { for i := range m.alerts { if m.alerts[i].Title == title && !m.alerts[i].Resolved { @@ -314,7 +324,7 @@ func (m *Monitor) runHealthChecks() { m.triggerHealthCheckAlert(check, err) } else { // Resolve any existing alerts for this check - m.ResolveAlert(fmt.Sprintf("health_check_%s", check.Name)) + m.resolveAlertLocked(fmt.Sprintf("health_check_%s", check.Name)) } } @@ -322,13 +332,13 @@ func (m *Monitor) runHealthChecks() { m.consecutiveFails = 0 if !m.isHealthy { m.isHealthy = true - m.ResolveAlert("system_unhealthy") + m.resolveAlertLocked("system_unhealthy") } } else { m.consecutiveFails++ if m.consecutiveFails >= m.config.AlertThresholds.MaxConsecutiveFailures { m.isHealthy = false - m.TriggerAlert(AlertLevelCritical, "system_unhealthy", + m.triggerAlertLocked(AlertLevelCritical, "system_unhealthy", fmt.Sprintf("System unhealthy after %d consecutive failures", m.consecutiveFails), map[string]interface{}{ "consecutive_failures": m.consecutiveFails, diff --git a/internal/wal/monitor_test.go b/internal/wal/monitor_test.go new file mode 100644 index 0000000..1d4801c --- /dev/null +++ b/internal/wal/monitor_test.go @@ -0,0 +1,119 @@ +package wal + +import ( + "errors" + "sync/atomic" + "testing" + "time" +) + +// Exercise Start's actual ticker and Stop's WaitGroup, rather than just calling +// alert helpers. Before the fix both successful checks and the critical-failure +// threshold re-entered the monitor mutex, preventing Stop from ever returning. +func TestMonitorTickerStopsAfterHealthChecks(t *testing.T) { + for _, failing := range []bool{false, true} { + name := "healthy" + if failing { + name = "critical_failure" + } + t.Run(name, func(t *testing.T) { + m := NewMonitor(NewMetrics(), MonitorConfig{ + HealthCheckInterval: 5 * time.Millisecond, + AlertThresholds: AlertThresholds{MaxConsecutiveFailures: 1}, + }) + var calls atomic.Int32 + check := HealthCheck{Name: "regression", Critical: true, Timeout: time.Second, + Check: func() error { + calls.Add(1) + if failing { + return errors.New("unavailable") + } + return nil + }, + } + if failing { + m.healthChecks = []HealthCheck{check} + } else { + // Keep the default healthy checks: their first successful alert + // resolution caused the observed released-binary deadlock. + m.AddHealthCheck(check) + } + m.Start() + defer m.cancel() + deadline := time.Now().Add(2 * time.Second) + for calls.Load() < 2 && time.Now().Before(deadline) { + time.Sleep(time.Millisecond) + } + if calls.Load() < 2 { + t.Fatal("health ticker did not complete multiple rounds") + } + stopped := make(chan struct{}) + go func() { m.Stop(); close(stopped) }() + select { + case <-stopped: + case <-time.After(2 * time.Second): + t.Fatal("Stop blocked after health-check ticks") + } + }) + } +} + +func TestMonitorHealthFailureAndRecovery(t *testing.T) { + m := NewMonitor(NewMetrics(), MonitorConfig{ + AlertThresholds: AlertThresholds{MaxConsecutiveFailures: 2}, + }) + defer m.cancel() + failure := errors.New("unavailable") + m.healthChecks = []HealthCheck{{Name: "storage", Critical: true, Timeout: time.Second, + Check: func() error { return failure }, + }} + runRound := func() { + t.Helper() + done := make(chan struct{}) + go func() { m.runHealthChecks(); close(done) }() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("health check deadlocked while changing alert state") + } + } + runRound() + if !m.IsHealthy() || m.consecutiveFails != 1 { + t.Fatal("a single failure must not cross the configured threshold") + } + runRound() + if m.IsHealthy() || m.consecutiveFails != 2 { + t.Fatal("repeated critical failures must mark the monitor unhealthy") + } + var systemAlert bool + for _, alert := range m.GetActiveAlerts() { + if alert.Title == "system_unhealthy" && alert.Level == AlertLevelCritical { + systemAlert = true + } + } + if !systemAlert { + t.Fatal("missing critical system alert") + } + + failure = nil + runRound() + if !m.IsHealthy() || m.consecutiveFails != 0 { + t.Fatal("a successful round must restore healthy state") + } + for _, alert := range m.alerts { + if alert.Title == "system_unhealthy" && (!alert.Resolved || alert.ResolvedAt.IsZero()) { + t.Fatal("recovery did not resolve the system alert") + } + } + // Existing semantics resolve one matching alert per round. A second healthy + // round clears the second recorded failure for this check. + runRound() + if len(m.GetActiveAlerts()) != 0 { + t.Fatal("successful rounds left unresolved health-check alerts") + } + m.TriggerAlert(AlertLevelInfo, "manual", "verification", nil) + m.ResolveAlert("manual") + if len(m.GetActiveAlerts()) != 0 { + t.Fatal("public alert methods no longer resolve alerts") + } +} diff --git a/npm/package.json b/npm/package.json index 23d03d0..06b6a7b 100644 --- a/npm/package.json +++ b/npm/package.json @@ -1,6 +1,6 @@ { "name": "argonctl", - "version": "2.1.0", + "version": "2.1.1", "mcpName": "io.github.argon-lab/argon", "description": "Git for MongoDB: branch, time-travel, merge and undo - CLI", "keywords": [ diff --git a/server.json b/server.json index a43c020..b4d32f4 100644 --- a/server.json +++ b/server.json @@ -6,12 +6,12 @@ "url": "https://github.com/argon-lab/argon", "source": "github" }, - "version": "2.1.0", + "version": "2.1.1", "packages": [ { "registryType": "npm", "identifier": "argonctl", - "version": "2.1.0", + "version": "2.1.1", "transport": { "type": "stdio" },