Skip to content

Commit

Permalink
http2: avoid corruption in priority write scheduler
Browse files Browse the repository at this point in the history
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
  • Loading branch information
neild committed May 30, 2024
1 parent 0d515a5 commit 5608279
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
4 changes: 2 additions & 2 deletions http2/writesched_priority.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 34 additions & 0 deletions http2/writesched_priority_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 5608279

Please sign in to comment.