Skip to content

Commit 1f9334f

Browse files
bootjpclaude
andcommitted
Fix truncateValue []byte marker, bounded cleanup wait, mock Close race
- Add "...(truncated)" marker to []byte path in truncateValue - Add bounded timeout (5s) to cleanup() wait on forwardMessages to prevent hanging on stuck client sockets - Guard mockDetachedConn.Close() with mutex to fix data race under -race Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 721087b commit 1f9334f

3 files changed

Lines changed: 20 additions & 4 deletions

File tree

proxy/pubsub.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"log/slog"
88
"strings"
99
"sync"
10+
"time"
1011

1112
"github.com/redis/go-redis/v9"
1213
"github.com/tidwall/redcon"
@@ -28,6 +29,10 @@ const (
2829
cmdDiscard = "DISCARD"
2930
cmdPing = "PING"
3031
cmdQuit = "QUIT"
32+
33+
// cleanupFwdTimeout bounds the wait for forwardMessages to exit during cleanup.
34+
// If the client socket is stuck, we don't want to block indefinitely.
35+
cleanupFwdTimeout = 5 * time.Second
3136
)
3237

3338
// pubsubSession manages a single client's detached connection.
@@ -77,7 +82,13 @@ func (s *pubsubSession) cleanup() {
7782
}
7883
s.mu.Unlock()
7984
if s.fwdDone != nil {
80-
<-s.fwdDone
85+
// Bounded wait: if forwardMessages is stuck on a slow/dead client socket,
86+
// don't block cleanup indefinitely.
87+
select {
88+
case <-s.fwdDone:
89+
case <-time.After(cleanupFwdTimeout):
90+
s.logger.Warn("forwardMessages did not exit within timeout, proceeding with cleanup")
91+
}
8192
}
8293
s.dconn.Close()
8394
}

proxy/pubsub_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,12 @@ func (m *mockDetachedConn) ReadCommand() (redcon.Command, error) {
5252
}
5353

5454
func (m *mockDetachedConn) Flush() error { return nil }
55-
func (m *mockDetachedConn) Close() error { m.closed = true; return nil }
55+
func (m *mockDetachedConn) Close() error {
56+
m.mu.Lock()
57+
defer m.mu.Unlock()
58+
m.closed = true
59+
return nil
60+
}
5661

5762
func (m *mockDetachedConn) RemoteAddr() string { return "127.0.0.1:9999" }
5863

proxy/sentry.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,9 @@ func truncateValue(v any) string {
149149
s = tv
150150
case []byte:
151151
if len(tv) > maxSentryValueLen {
152-
tv = tv[:maxSentryValueLen]
152+
return string(tv[:maxSentryValueLen]) + "...(truncated)"
153153
}
154-
s = string(tv)
154+
return string(tv)
155155
default:
156156
s = fmt.Sprintf("%v", v)
157157
}

0 commit comments

Comments
 (0)