From e70a9fca7cc1fe687de932323917861a3a85867f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E6=99=BA=E8=B6=850668000959?= Date: Tue, 2 Jun 2026 19:45:31 +0800 Subject: [PATCH] fix(tools): use sync.Once for thread-safe Stop() in SessionManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Stop() method previously used a select/default pattern which was not safe under concurrent calls — two goroutines could both pass the check and attempt to close the same channel, causing a panic. Replace with sync.Once to guarantee exactly-once close semantics, matching the documented contract of being safe for concurrent use. Review feedback: afjcjsbx --- pkg/tools/session.go | 14 ++++++-------- pkg/tools/shell_test.go | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/pkg/tools/session.go b/pkg/tools/session.go index 65aa924fd..2b03df350 100644 --- a/pkg/tools/session.go +++ b/pkg/tools/session.go @@ -172,6 +172,7 @@ type SessionManager struct { mu sync.RWMutex sessions map[string]*ProcessSession stopCh chan struct{} + stopOnce sync.Once } func NewSessionManager() *SessionManager { @@ -197,16 +198,13 @@ func NewSessionManager() *SessionManager { return sm } -// Stop shuts down the background cleanup goroutine. Safe to call multiple times. -// After Stop returns, the SessionManager is still usable — only the cleanup -// goroutine is terminated. +// Stop shuts down the background cleanup goroutine. Safe to call multiple +// times from concurrent goroutines. After Stop returns, the SessionManager +// is still usable — only the cleanup goroutine is terminated. func (sm *SessionManager) Stop() { - select { - case <-sm.stopCh: - // already closed - default: + sm.stopOnce.Do(func() { close(sm.stopCh) - } + }) } // cleanupOldSessions removes sessions that are done and older than 30 minutes diff --git a/pkg/tools/shell_test.go b/pkg/tools/shell_test.go index 4ad1f4ee8..83807dac6 100644 --- a/pkg/tools/shell_test.go +++ b/pkg/tools/shell_test.go @@ -1,4 +1,4 @@ -package tools +package tools import ( "context"