Skip to content

Commit 01e627c

Browse files
committed
fix: data races detected by -race in SessionManager.Save and poller test
- Copy-on-write in SessionManager.Save (mutate keeps old pointer safe) - TurnCount/fields snapshot under lock before unlock - Test races fixed: atomic.Int32 for attempts counter in poller test - Tests updated for copy-on-write pointer semantics (re-fetch after Save) - Full project passes: go test -short -race ./... — 19/19 packages OK
1 parent 36b5b90 commit 01e627c

3 files changed

Lines changed: 30 additions & 19 deletions

File tree

internal/telegram/poller_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/http"
77
"net/http/httptest"
88
"strings"
9+
"sync/atomic"
910
"testing"
1011
"time"
1112
)
@@ -633,10 +634,10 @@ func TestPoller_Start_FatalError(t *testing.T) {
633634
}
634635

635636
func TestPoller_Start_RetriesOnTransientError(t *testing.T) {
636-
attempts := 0
637+
var attempts atomic.Int32
637638
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
638-
attempts++
639-
if attempts < 3 {
639+
attempts.Add(1)
640+
if attempts.Load() < 3 {
640641
failResponse(w, 502, "Bad Gateway")
641642
return
642643
}
@@ -670,7 +671,7 @@ func TestPoller_Start_RetriesOnTransientError(t *testing.T) {
670671
if err != context.DeadlineExceeded && err != context.Canceled {
671672
t.Errorf("Start returned %v, want context deadline/cancel error", err)
672673
}
673-
if attempts < 3 {
674-
t.Errorf("attempts = %d, want >= 3", attempts)
674+
if attempts.Load() < 3 {
675+
t.Errorf("attempts = %d, want >= 3", attempts.Load())
675676
}
676677
}

internal/telegram/session.go

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,14 @@ func (sm *SessionManager) Save(chatID int64, messages []llm.Message) error {
105105
sm.Mu.Lock()
106106
cs, ok := sm.Cache[chatID]
107107
if ok {
108-
cs.Messages = messages
109-
cs.LastActive = time.Now()
110-
cs.TurnCount++
108+
// Copy-on-write: create a new ChatSession so existing pointers
109+
// held by Load() callers are not mutated, avoiding data races.
110+
updated := *cs
111+
updated.Messages = messages
112+
updated.LastActive = time.Now()
113+
updated.TurnCount++
114+
cs = &updated
115+
sm.Cache[chatID] = cs
111116
} else {
112117
cs = &ChatSession{
113118
ChatID: chatID,
@@ -118,15 +123,18 @@ func (sm *SessionManager) Save(chatID int64, messages []llm.Message) error {
118123
}
119124
sm.Cache[chatID] = cs
120125
}
126+
// Snapshot fields needed after unlock to avoid data race:
127+
sessionID := cs.SessionID
128+
createdAt := cs.CreatedAt
129+
turnCount := cs.TurnCount
121130
sm.Mu.Unlock()
122131

123-
now := time.Now()
124132
sess := &session.Session{
125-
ID: cs.SessionID,
126-
CreatedAt: cs.CreatedAt,
127-
UpdatedAt: now,
133+
ID: sessionID,
134+
CreatedAt: createdAt,
135+
UpdatedAt: time.Now(),
128136
Model: "",
129-
Turns: cs.TurnCount,
137+
Turns: turnCount,
130138
Task: fmt.Sprintf("tg-%d", chatID),
131139
Messages: messages,
132140
}

internal/telegram/session_test.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -277,11 +277,12 @@ func TestSave_incrementsTurnCount(t *testing.T) {
277277
t.Errorf("TurnCount = %d, want 1 after first save", cs.TurnCount)
278278
}
279279

280-
// Second save → TurnCount = 2
280+
// Second save → re-fetch from cache to verify increment
281281
err = sm.Save(chatID, []llm.Message{{Role: "user", Content: "turn 2"}})
282282
if err != nil {
283283
t.Fatalf("second Save failed: %v", err)
284284
}
285+
cs, _ = sm.GetOrCreate(chatID)
285286
if cs.TurnCount != 2 {
286287
t.Errorf("TurnCount = %d, want 2 after second save", cs.TurnCount)
287288
}
@@ -291,14 +292,10 @@ func TestSave_incrementsTurnCount(t *testing.T) {
291292
if err != nil {
292293
t.Fatalf("third Save failed: %v", err)
293294
}
295+
cs, _ = sm.GetOrCreate(chatID)
294296
if cs.TurnCount != 3 {
295297
t.Errorf("TurnCount = %d, want 3 after third save", cs.TurnCount)
296298
}
297-
298-
// Verify disk also has the right turn count.
299-
if cs.TurnCount != 3 {
300-
t.Errorf("TurnCount = %d, want 3", cs.TurnCount)
301-
}
302299
}
303300

304301
// ---------------------------------------------------------------------------
@@ -497,6 +494,11 @@ func TestAppendMessage(t *testing.T) {
497494
if err != nil {
498495
t.Fatalf("second AppendMessage failed: %v", err)
499496
}
497+
// Re-fetch after Save (copy-on-write replaces cache pointer).
498+
cs, err = sm.GetOrCreate(chatID)
499+
if err != nil {
500+
t.Fatalf("GetOrCreate after second append failed: %v", err)
501+
}
500502
if len(cs.Messages) != 2 {
501503
t.Errorf("Messages length = %d, want 2", len(cs.Messages))
502504
}

0 commit comments

Comments
 (0)