From 35e2d1cf2316766f1ab63d8260c173ccb21a4d85 Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Mon, 3 Jul 2017 16:34:30 +0100 Subject: [PATCH 1/3] Fix that Ctrie iterator did not return entries from tNodes. Fixes #171 --- trie/ctrie/ctrie.go | 4 +++- trie/ctrie/ctrie_test.go | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/trie/ctrie/ctrie.go b/trie/ctrie/ctrie.go index b95425b..a167f76 100644 --- a/trie/ctrie/ctrie.go +++ b/trie/ctrie/ctrie.go @@ -1,5 +1,5 @@ /* -Copyright 2015 Workiva, LLC +Copyright 2015-2017 Workiva, LLC and others. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -405,6 +405,8 @@ func (c *Ctrie) traverse(i *iNode, ch chan<- *Entry, cancel <-chan struct{}) err return errCanceled } } + case main.tNode != nil: + ch <- main.tNode.Entry } return nil } diff --git a/trie/ctrie/ctrie_test.go b/trie/ctrie/ctrie_test.go index a3fa561..0163984 100644 --- a/trie/ctrie/ctrie_test.go +++ b/trie/ctrie/ctrie_test.go @@ -341,6 +341,24 @@ func TestIterator(t *testing.T) { assert.False(ok) } +// TestIteratorCoversTNodes reproduces the scenario of a bug where tNodes weren't being traversed. +func TestIteratorCoversTNodes(t *testing.T) { + assert := assert.New(t) + ctrie := New(mockHashFactory) + // Add a pair of keys that collide (because we're using the mock hash). + ctrie.Insert([]byte("a"), true) + ctrie.Insert([]byte("b"), true) + // Delete one key, leaving exactly one sNode in the cNode. This will + // trigger creation of a tNode. + ctrie.Remove([]byte("b")) + seenKeys := map[string]bool{} + for entry := range ctrie.Iterator(nil) { + seenKeys[string(entry.Key)] = true + } + assert.Contains(seenKeys, "a", "Iterator did not return 'a'.") + assert.Len(seenKeys, 1) +} + func TestSize(t *testing.T) { ctrie := New(nil) for i := 0; i < 10; i++ { From d2b2290a45db4d6aa0e1b0beb0045a9dc7094aff Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Thu, 20 Jul 2017 11:57:23 +0100 Subject: [PATCH 2/3] Ensure traverse is cancelable even if it's sending a tNode. --- trie/ctrie/ctrie.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/trie/ctrie/ctrie.go b/trie/ctrie/ctrie.go index a167f76..8a9a9c3 100644 --- a/trie/ctrie/ctrie.go +++ b/trie/ctrie/ctrie.go @@ -406,7 +406,11 @@ func (c *Ctrie) traverse(i *iNode, ch chan<- *Entry, cancel <-chan struct{}) err } } case main.tNode != nil: - ch <- main.tNode.Entry + select { + case ch <- main.tNode.Entry: + case <-cancel: + return errCanceled + } } return nil } From b4ec1df61a85b06ecd47d331e081250781675c9c Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Fri, 28 Jul 2017 10:13:55 +0100 Subject: [PATCH 3/3] Revert copyright change. --- trie/ctrie/ctrie.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trie/ctrie/ctrie.go b/trie/ctrie/ctrie.go index 8a9a9c3..0f21ce2 100644 --- a/trie/ctrie/ctrie.go +++ b/trie/ctrie/ctrie.go @@ -1,5 +1,5 @@ /* -Copyright 2015-2017 Workiva, LLC and others. +Copyright 2015 Workiva, LLC Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License.