bound the tree rebuild against a cyclic channel graph
rebuild_rec followed parent/child links with no depth limit, so a channel graph in which a channel is its own ancestor recursed until the process ran out of memory. Cap the depth and give the whole rebuild a single node budget. Accumulating into one slice rather than returning a new slice per node makes that budget apply to the tree as a whole instead of to each level, and removes an allocation per node. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
314b838e33
commit
07c923c4c9
+25
-15
@@ -89,10 +89,10 @@ func (t *Tree) rebuild(preserveActive bool, sameItem func(previous, current Tree
|
|||||||
previousLine := t.activeLine
|
previousLine := t.activeLine
|
||||||
lines := []renderedTreeItem{}
|
lines := []renderedTreeItem{}
|
||||||
for _, item := range t.Generator(nil) {
|
for _, item := range t.Generator(nil) {
|
||||||
children := t.rebuild_rec(item, 0)
|
if len(lines) >= maxTreeLines {
|
||||||
if children != nil {
|
break
|
||||||
lines = append(lines, children...)
|
|
||||||
}
|
}
|
||||||
|
lines = t.rebuild_rec(lines, item, 0)
|
||||||
}
|
}
|
||||||
t.lines = lines
|
t.lines = lines
|
||||||
if preserveActive {
|
if preserveActive {
|
||||||
@@ -113,21 +113,31 @@ func (t *Tree) rebuild(preserveActive bool, sameItem func(previous, current Tree
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (t *Tree) rebuild_rec(parent TreeItem, level int) []renderedTreeItem {
|
// A server is free to describe a channel graph in which a channel is its own
|
||||||
if parent == nil {
|
// ancestor. The generator follows parent/child links literally, so without
|
||||||
return nil
|
// these limits such a graph recurses until the process is out of memory.
|
||||||
}
|
// Real trees are orders of magnitude smaller than either bound.
|
||||||
lines := []renderedTreeItem{
|
const (
|
||||||
renderedTreeItem{
|
maxTreeDepth = 64
|
||||||
Level: level,
|
maxTreeLines = 100000
|
||||||
Item: parent,
|
)
|
||||||
},
|
|
||||||
|
// rebuild_rec appends parent and its descendants to lines. Accumulating into
|
||||||
|
// one slice keeps maxTreeLines a budget for the whole tree rather than for
|
||||||
|
// each level, and avoids building a slice per node.
|
||||||
|
func (t *Tree) rebuild_rec(lines []renderedTreeItem, parent TreeItem, level int) []renderedTreeItem {
|
||||||
|
if parent == nil || level >= maxTreeDepth || len(lines) >= maxTreeLines {
|
||||||
|
return lines
|
||||||
}
|
}
|
||||||
|
lines = append(lines, renderedTreeItem{
|
||||||
|
Level: level,
|
||||||
|
Item: parent,
|
||||||
|
})
|
||||||
for _, item := range t.Generator(parent) {
|
for _, item := range t.Generator(parent) {
|
||||||
children := t.rebuild_rec(item, level+1)
|
if len(lines) >= maxTreeLines {
|
||||||
if children != nil {
|
break
|
||||||
lines = append(lines, children...)
|
|
||||||
}
|
}
|
||||||
|
lines = t.rebuild_rec(lines, item, level+1)
|
||||||
}
|
}
|
||||||
return lines
|
return lines
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,65 @@
|
|||||||
|
package uiterm
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
// cyclicItem reports itself as its own child, standing in for a channel graph
|
||||||
|
// in which a channel is its own ancestor.
|
||||||
|
type cyclicItem struct{ name string }
|
||||||
|
|
||||||
|
func (i *cyclicItem) String() string { return i.name }
|
||||||
|
|
||||||
|
func (i *cyclicItem) TreeItemStyle(fg, bg Attribute, active bool) (Attribute, Attribute) {
|
||||||
|
return fg, bg
|
||||||
|
}
|
||||||
|
|
||||||
|
// Regression: rebuild_rec followed parent/child links with no depth limit, so
|
||||||
|
// a cyclic channel graph recursed until the process ran out of memory. A
|
||||||
|
// rebuild must now terminate and stay bounded.
|
||||||
|
func TestTreeRebuildTerminatesOnCyclicGraph(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
self := &cyclicItem{name: "loop"}
|
||||||
|
tree := Tree{
|
||||||
|
Generator: func(item TreeItem) []TreeItem {
|
||||||
|
return []TreeItem{self}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
done := make(chan struct{})
|
||||||
|
go func() {
|
||||||
|
tree.rebuild(false, nil)
|
||||||
|
close(done)
|
||||||
|
}()
|
||||||
|
|
||||||
|
select {
|
||||||
|
case <-done:
|
||||||
|
case <-timeoutAfterSeconds(10):
|
||||||
|
t.Fatal("rebuild did not terminate on a cyclic tree")
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(tree.lines) == 0 {
|
||||||
|
t.Fatal("expected the bounded rebuild to still produce lines")
|
||||||
|
}
|
||||||
|
if len(tree.lines) > maxTreeLines {
|
||||||
|
t.Fatalf("rebuild produced %d lines, above the %d cap",
|
||||||
|
len(tree.lines), maxTreeLines)
|
||||||
|
}
|
||||||
|
for _, line := range tree.lines {
|
||||||
|
if line.Level >= maxTreeDepth {
|
||||||
|
t.Fatalf("rebuild recursed to level %d, at or past the %d cap",
|
||||||
|
line.Level, maxTreeDepth)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func timeoutAfterSeconds(n int) <-chan struct{} {
|
||||||
|
ch := make(chan struct{})
|
||||||
|
go func() {
|
||||||
|
time.Sleep(time.Duration(n) * time.Second)
|
||||||
|
close(ch)
|
||||||
|
}()
|
||||||
|
return ch
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user