From 40da7fdb760c383d42a68f7652230ae36ba40dc0 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Wed, 15 Jul 2026 03:27:40 +0200 Subject: [PATCH] fix(xray): retry a failed pending-restart instead of dropping the config change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 30s cron consumed the need-restart flag with IsNeedRestartAndSetFalse before calling RestartXray and only logged a failure. If RestartXray failed early (a transient GetXrayConfig DB error) the old process kept running the old config, the crash detector saw a running process and never retried, and the flag stayed cleared — so an admin's saved change silently never reached the core. Move the consume/restart/retry into ApplyPendingRestart, which re-arms the flag on failure so the next tick retries. --- internal/web/service/xray.go | 14 ++++++++++++++ internal/web/service/xray_restart_test.go | 23 +++++++++++++++++++++++ internal/web/web.go | 7 +------ 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/internal/web/service/xray.go b/internal/web/service/xray.go index cd0786abd..2a2a0368f 100644 --- a/internal/web/service/xray.go +++ b/internal/web/service/xray.go @@ -1188,6 +1188,20 @@ func (s *XrayService) IsNeedRestartAndSetFalse() bool { return isNeedXrayRestart.CompareAndSwap(true, false) } +// ApplyPendingRestart consumes the need-restart flag and restarts Xray. If the +// restart fails (for example GetXrayConfig hits a transient DB error and leaves +// the old process running), it re-arms the flag so the next tick retries instead +// of silently dropping the pending config change. +func (s *XrayService) ApplyPendingRestart() { + if !s.IsNeedRestartAndSetFalse() { + return + } + if err := s.RestartXray(false); err != nil { + logger.Error("restart xray failed:", err) + s.SetToNeedRestart() + } +} + // DidXrayCrash checks if Xray crashed by verifying it's not running and wasn't manually stopped. func (s *XrayService) DidXrayCrash() bool { return !s.IsXrayRunning() && !isManuallyStopped.Load() diff --git a/internal/web/service/xray_restart_test.go b/internal/web/service/xray_restart_test.go index 3c39871d5..aa0611612 100644 --- a/internal/web/service/xray_restart_test.go +++ b/internal/web/service/xray_restart_test.go @@ -21,3 +21,26 @@ func TestRestartXrayRespectsManualStop(t *testing.T) { t.Fatal("a non-forced restart cleared a deliberate manual stop and would revive xray") } } + +// When the pending-restart reconcile consumes the need-restart flag but the +// restart itself fails, the flag must be re-armed so the config change is +// retried rather than silently dropped. +func TestApplyPendingRestartReArmsFlagOnFailure(t *testing.T) { + setupSettingTestDB(t) + if err := (&SettingService{}).saveSetting("xrayTemplateConfig", "{ not valid json"); err != nil { + t.Fatalf("seed template: %v", err) + } + t.Cleanup(func() { + isManuallyStopped.Store(false) + isNeedXrayRestart.Store(false) + }) + isManuallyStopped.Store(false) + + svc := &XrayService{} + svc.SetToNeedRestart() + svc.ApplyPendingRestart() + + if !isNeedXrayRestart.Load() { + t.Fatal("a failed restart must re-arm the need-restart flag so the pending config change is retried") + } +} diff --git a/internal/web/web.go b/internal/web/web.go index d19d28238..76f8927ec 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -314,12 +314,7 @@ func (s *Server) startTask(restartXray bool) { // Check if xray needs to be restarted every 30 seconds _, _ = s.cron.AddFunc(cadenceXrayRestart, func() { - if s.xrayService.IsNeedRestartAndSetFalse() { - err := s.xrayService.RestartXray(false) - if err != nil { - logger.Error("restart xray failed:", err) - } - } + s.xrayService.ApplyPendingRestart() }) go func() {