Skip to content

Commit 455ecf2

Browse files
committed
Bound pending cancel notification queue
Change-Id: I1e6195bae6969b5e07b532c5f042244530eb9a9e Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent fccab8e commit 455ecf2

2 files changed

Lines changed: 45 additions & 1 deletion

File tree

connection.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ func (c *Connection) loggerOrDefault() *slog.Logger {
114114
const (
115115
maxCanonicalJSONRPCIDKeyLen = 4096
116116
maxCanonicalJSONRPCIDAbsExp10 = 4096
117+
maxPendingCancelRequests = 1024
117118
)
118119

119120
var (
@@ -666,10 +667,20 @@ func (c *Connection) sendCancelRequest(idKey string) {
666667
default:
667668
}
668669

670+
queueFull := false
669671
c.mu.Lock()
670-
c.pendingCancelRequest = append(c.pendingCancelRequest, idKey)
672+
if len(c.pendingCancelRequest) >= maxPendingCancelRequests {
673+
queueFull = true
674+
} else {
675+
c.pendingCancelRequest = append(c.pendingCancelRequest, idKey)
676+
}
671677
c.mu.Unlock()
672678

679+
if queueFull {
680+
c.loggerOrDefault().Debug("dropping $/cancel_request due to full queue", "queue_len", maxPendingCancelRequests)
681+
return
682+
}
683+
673684
select {
674685
case c.cancelRequestSignal <- struct{}{}:
675686
default:

connection_cancel_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,39 @@ func TestConnectionOutboundCancelRequest_DoesNotBlockWhenPeerStopsReading(t *tes
586586
}
587587
}
588588

589+
func TestConnectionSendCancelRequest_BoundsPendingQueue(t *testing.T) {
590+
baseCtx, baseCancel := context.WithCancelCause(context.Background())
591+
defer baseCancel(nil)
592+
593+
c := &Connection{
594+
pending: make(map[string]*pendingResponse),
595+
inflight: make(map[string]context.CancelCauseFunc),
596+
cancelRequestSignal: make(chan struct{}, 1),
597+
ctx: baseCtx,
598+
cancel: baseCancel,
599+
}
600+
601+
for i := 0; i < maxPendingCancelRequests+128; i++ {
602+
c.sendCancelRequest(fmt.Sprintf("%d", i))
603+
}
604+
605+
c.mu.Lock()
606+
defer c.mu.Unlock()
607+
608+
if len(c.pendingCancelRequest) != maxPendingCancelRequests {
609+
t.Fatalf("expected pending cancel queue length %d, got %d", maxPendingCancelRequests, len(c.pendingCancelRequest))
610+
}
611+
612+
if got := c.pendingCancelRequest[0]; got != "0" {
613+
t.Fatalf("expected queue to retain earliest id when full, got first id %q", got)
614+
}
615+
616+
expectedLast := fmt.Sprintf("%d", maxPendingCancelRequests-1)
617+
if got := c.pendingCancelRequest[len(c.pendingCancelRequest)-1]; got != expectedLast {
618+
t.Fatalf("expected queue to drop ids beyond capacity, got last id %q want %q", got, expectedLast)
619+
}
620+
}
621+
589622
func TestConnectionWaitForResponse_PeerDisconnectWinsOverDerivedContextCancel(t *testing.T) {
590623
const iterations = 64
591624

0 commit comments

Comments
 (0)