From 56082791fe602007bf2068450f13b8876c2a23dd Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Wed, 29 May 2024 14:04:31 -0700 Subject: [PATCH] http2: avoid corruption in priority write scheduler When removing a stream containing children in the priority tree, it was possible for some children to not be correctly moved to the parent of the removed stream. Fixes golang/go#66514 Change-Id: Ie8a8743a6213a6b1a2426e023111878afff78f9e Reviewed-on: https://go-review.googlesource.com/c/net/+/589255 LUCI-TryBot-Result: Go LUCI Reviewed-by: Jonathan Amsterdam --- http2/writesched_priority.go | 4 ++-- http2/writesched_priority_test.go | 34 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/http2/writesched_priority.go b/http2/writesched_priority.go index 0a242c669..f6783339d 100644 --- a/http2/writesched_priority.go +++ b/http2/writesched_priority.go @@ -443,8 +443,8 @@ func (ws *priorityWriteScheduler) addClosedOrIdleNode(list *[]*priorityNode, max } func (ws *priorityWriteScheduler) removeNode(n *priorityNode) { - for k := n.kids; k != nil; k = k.next { - k.setParent(n.parent) + for n.kids != nil { + n.kids.setParent(n.parent) } n.setParent(nil) delete(ws.nodes, n.id) diff --git a/http2/writesched_priority_test.go b/http2/writesched_priority_test.go index b579ef987..5aad057be 100644 --- a/http2/writesched_priority_test.go +++ b/http2/writesched_priority_test.go @@ -562,3 +562,37 @@ func TestPriorityRstStreamOnNonOpenStreams(t *testing.T) { t.Error(err) } } + +// https://go.dev/issue/66514 +func TestPriorityIssue66514(t *testing.T) { + addDep := func(ws *priorityWriteScheduler, child uint32, parent uint32) { + ws.AdjustStream(child, PriorityParam{ + StreamDep: parent, + Exclusive: false, + Weight: 16, + }) + } + + validateDepTree := func(ws *priorityWriteScheduler, id uint32, t *testing.T) { + for n := ws.nodes[id]; n != nil; n = n.parent { + if n.parent == nil { + if n.id != uint32(0) { + t.Errorf("detected nodes not parented to 0") + } + } + } + } + + ws := NewPriorityWriteScheduler(nil).(*priorityWriteScheduler) + + // Root entry + addDep(ws, uint32(1), uint32(0)) + addDep(ws, uint32(3), uint32(1)) + addDep(ws, uint32(5), uint32(1)) + + for id := uint32(7); id < uint32(100); id += uint32(4) { + addDep(ws, id, id-uint32(4)) + addDep(ws, id+uint32(2), id-uint32(4)) + validateDepTree(ws, id, t) + } +}