Skip to content

Commit 36ee348

Browse files
joamakidylandreimerink
authored andcommitted
part: Add missing closing of root watch channel
The commit f8e6b9e changed the implementation not to always have a zero-prefix node4 at the root. This change unfortunately missed marking the replaced root's watch channel for closing. This fixes the issue and adds a validation to make sure any watch channel that no longer is referenced in the new tree is closed. === RUN Test_replaceRoot node4[61]: 61 -> 1 (L:0x4000041b00 W:0x4000014a80 true) (N:0x400012c500, W:0x4000014bd0 false) leaf[62]: 6162 -> 3 (L:0x4000041b30 W:0x4000014b60 false) (N:0x4000041b30, W:0x4000014b60 false) --- leaf[6162]: 6162 -> 3 (L:0x4000041bf0 W:0x4000014b60 false) (N:0x4000041bf0, W:0x4000014b60 false) --- FAIL: Test_replaceRoot (0.00s) panic: dropped watch channel 0x4000014bd0 at depth 0 not closed [recovered] panic: dropped watch channel 0x4000014bd0 at depth 0 not closed Fixes: f8e6b9e ("part: Remove the unnecessary empty root node") Signed-off-by: Jussi Maki <jussi@isovalent.com>
1 parent 6295bf1 commit 36ee348

File tree

1 file changed

+56
-0
lines changed

1 file changed

+56
-0
lines changed

part/txn.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,9 @@ func (txn *Txn[T]) Notify() {
194194
close(txn.rootWatch)
195195
txn.rootWatch = nil
196196
}
197+
if !txn.opts.rootOnlyWatch() {
198+
validateRemovedWatches(txn.oldRoot, txn.root)
199+
}
197200
}
198201

199202
// PrintTree to the standard output. For debugging.
@@ -426,9 +429,15 @@ func (txn *Txn[T]) delete(root *header[T], key []byte) (oldValue T, hadOld bool,
426429
if target == root {
427430
switch {
428431
case root.isLeaf() || root.size() == 0:
432+
if root.watch != nil {
433+
txn.watches[root.watch] = struct{}{}
434+
}
429435
// Root is a leaf or node without children
430436
newRoot = nil
431437
case root.size() == 1:
438+
if root.watch != nil {
439+
txn.watches[root.watch] = struct{}{}
440+
}
432441
// Root is a non-leaf node with single child. We can replace
433442
// the root with the child.
434443
child := root.children()[0]
@@ -471,6 +480,9 @@ func (txn *Txn[T]) delete(root *header[T], key []byte) (oldValue T, hadOld bool,
471480
} else {
472481
// The target node is a leaf node or a non-leaf node without any
473482
// children. We can just drop it from the parent.
483+
if this.node.watch != nil {
484+
txn.watches[this.node.watch] = struct{}{}
485+
}
474486
parent.node = txn.removeChild(parent.node, this.index)
475487
}
476488
index--
@@ -675,3 +687,47 @@ func validateTree[T any](node *header[T], parents []*header[T], watches map[chan
675687
}
676688
}
677689
}
690+
691+
func validateRemovedWatches[T any](oldRoot *header[T], newRoot *header[T]) {
692+
if !runValidation {
693+
return
694+
}
695+
696+
var collectWatches func(depth int, watches map[<-chan struct{}]int, node *header[T])
697+
collectWatches = func(depth int, watches map[<-chan struct{}]int, node *header[T]) {
698+
if node == nil {
699+
return
700+
}
701+
if node.watch == nil {
702+
panic("nil watch channel")
703+
}
704+
watches[node.watch] = depth
705+
if leaf := node.getLeaf(); leaf != nil && !node.isLeaf() {
706+
watches[leaf.watch] = depth
707+
}
708+
for _, child := range node.children() {
709+
if child != nil {
710+
collectWatches(depth+1, watches, child)
711+
}
712+
}
713+
}
714+
oldWatches := map[<-chan struct{}]int{}
715+
collectWatches(0, oldWatches, oldRoot)
716+
newWatches := map[<-chan struct{}]int{}
717+
collectWatches(0, newWatches, newRoot)
718+
719+
// Any nodes that are not part of the new tree must have their watch channels closed.
720+
for watch := range newWatches {
721+
delete(oldWatches, watch)
722+
}
723+
for watch, depth := range oldWatches {
724+
select {
725+
case <-watch:
726+
default:
727+
oldRoot.printTree(0)
728+
fmt.Println("---")
729+
newRoot.printTree(0)
730+
panic(fmt.Sprintf("dropped watch channel %p at depth %d not closed", watch, depth))
731+
}
732+
}
733+
}

0 commit comments

Comments
 (0)