diff --git a/inc/cvc/state/state.h b/inc/cvc/state/state.h index 14acd1b60..e80708525 100644 --- a/inc/cvc/state/state.h +++ b/inc/cvc/state/state.h @@ -454,10 +454,13 @@ class state : public boost::enable_shared_from_this { // Clear this node's value, data, comment and hidden flag. // - // resetChildren = true (the default) recurses, resetting the subtree in - // place and keeping every node. + // resetChildren = true (the default) resets the subtree in place and keeps + // every node: a node is cleared before the nodes under it and, with + // fireCallbacks, touched after them, a child's whole subtree before the next + // child's. A loop, not a call per level, and no lock is held while a touch + // signals. // - // resetChildren = false DESTROYS the children instead of recursing. + // resetChildren = false DESTROYS the children instead. // Beware: operator() returns state&, never the owning state_ptr, so the // parent's child map holds the only reference and dropping it deletes the // whole subtree. Any state& a caller still holds into that subtree @@ -466,6 +469,12 @@ class state : public boost::enable_shared_from_this { void reset(bool resetChildren = true, bool fireCallbacks = true); // converting to and from a boost property tree. Useful for saving and restoring state. + // + // ptree() is one level: an entry per node of this subtree, keyed by its full name from the + // root, in pre-order (children in name order), holding its value, or "" for a node below this + // one with neither a value nor children. Values only, not data(). ptree(pt) sets each + // non-empty value at its key, relative to this node, at every nesting level, so it also loads + // the nested trees older versions wrote. json(), save() and restore() are the same as JSON. boost::property_tree::ptree ptree(); operator boost::property_tree::ptree() { return ptree(); } void ptree(const boost::property_tree::ptree &); @@ -479,6 +488,10 @@ class state : public boost::enable_shared_from_this { void save(const std::string &filename); void restore(const std::string &filename); + // Call func with the full name of this node, then of each descendant whose full name matches + // `re` (all of them when it is empty), each once, in pre-order, children in name order. A + // visited node's traverseEnter fires before func sees it and its traverseExit after the nodes + // under it are visited. Descendants under a node that does not match are still visited. void traverse(traversal_unary_func func, const std::string &re = std::string()); signal traverseEnter; signal traverseExit; diff --git a/src/cvc/state/state.cpp b/src/cvc/state/state.cpp index ee63b89da..ec4476abc 100644 --- a/src/cvc/state/state.cpp +++ b/src/cvc/state/state.cpp @@ -579,38 +579,84 @@ void state::touch() { // 03/30/2012 -- Joe R. -- Resetting comment. void state::reset(bool resetChildren, bool fireCallbacks) { boost::this_thread::interruption_point(); - { - boost::mutex::scoped_lock lock(_mutex); - _value = std::string(); - _valueTypeName = std::string(); - _data = boost::any(); - _comment = std::string(); - _hidden = false; - _initialized = false; - if (resetChildren) { - BOOST_FOREACH (child_map::value_type val, _children) - val.second->reset(resetChildren, fireCallbacks); - } else { - // NOT "reset everything but the children" -- this DESTROYS them. - // - // Nothing else owns a child: operator() hands out state&, never the - // owning state_ptr, so dropping the map runs ~state over the whole - // subtree and invalidates every reference a caller still holds: - // - // state &s = root("a.b"); - // root("a").reset(false); // s dangles from here on - // - // A later operator()("a.b") therefore does not return the old node; - // it silently creates a fresh, uninitialized one. That is exactly - // what the reset(false) tests observe when they check initialized(). - // Only tests call this today. A caller that genuinely wants - // detach-and-keep must be handed the state_ptrs rather than having - // them dropped here. - _children.clear(); + // Clears a node's own fields. The caller holds its lock. + const auto clear = [](state &n) { + n._value = std::string(); + n._valueTypeName = std::string(); + n._data = boost::any(); + n._comment = std::string(); + n._hidden = false; + n._initialized = false; + }; + + if (!resetChildren) { + // NOT "reset everything but the children" -- this DESTROYS them. + // + // Nothing else owns a child: operator() hands out state&, never the + // owning state_ptr, so dropping the map runs ~state over the whole + // subtree and invalidates every reference a caller still holds: + // + // state &s = root("a.b"); + // root("a").reset(false); // s dangles from here on + // + // A later operator()("a.b") therefore does not return the old node; + // it silently creates a fresh, uninitialized one. That is exactly + // what the reset(false) tests observe when they check initialized(). + // Only tests call this today. A caller that genuinely wants + // detach-and-keep must be handed the state_ptrs rather than having + // them dropped here. + // + // The map is taken out under the lock and dropped after it, so the subtree's destroyed() + // slots do not run under this node's lock. + child_map doomed; + { + boost::mutex::scoped_lock lock(_mutex); + clear(*this); + doomed.swap(_children); } + doomed.clear(); + if (fireCallbacks) + touch(); + return; } - if (fireCallbacks) { - touch(); + + // The subtree in place, in the order the recursion had: a node is cleared before the nodes + // under it and touched after them, a child's whole subtree before the next child's. With a heap + // stack of levels, not a call per level, and no lock held while signaling: each node is cleared + // and its children snapshotted under its own lock, released before going down. The recursion + // held every ancestor's lock while touching, so a slot that read an ancestor or looked its node + // up from the root (the sync adapter's do) deadlocked. Each level holds its node, so a touch's + // ancestors stay alive even if something drops them from the tree meanwhile. + struct level { + state *node; + state_ptr held; // null for this node: the caller holds it + std::vector kids; + std::size_t next = 0; + }; + const auto open = [&clear](state &n, state_ptr held) { + level l{&n, std::move(held), {}, 0}; + boost::mutex::scoped_lock lock(n._mutex); + clear(n); + l.kids.reserve(n._children.size()); + for (const child_map::value_type &kv : n._children) + if (kv.second) + l.kids.push_back(kv.second); + return l; + }; + std::vector stack; + stack.push_back(open(*this, state_ptr())); + while (!stack.empty()) { + level &l = stack.back(); + if (l.next < l.kids.size()) { + state_ptr kid = std::move(l.kids[l.next++]); + boost::this_thread::interruption_point(); + state &k = *kid; + stack.push_back(open(k, std::move(kid))); // invalidates `l` + continue; + } + if (fireCallbacks) + l.node->touch(); + stack.pop_back(); } } @@ -627,21 +673,52 @@ boost::property_tree::ptree state::ptree() { property_tree::ptree pt; boost::this_thread::interruption_point(); - _ctx.log(6, boost::str(boost::format("%s :: %s = %s\n") % BOOST_CURRENT_FUNCTION % fullName() % - value())); - - if (!value().empty()) - pt.push_back(property_tree::ptree::value_type(fullName(), property_tree::ptree(value()))); - { - boost::mutex::scoped_lock lock(_mutex); - BOOST_FOREACH (child_map::value_type val, _children) { - boost::this_thread::interruption_point(); - property_tree::ptree child_pt = val.second->ptree(); - pt.push_back(property_tree::ptree::value_type(val.second->fullName(), child_pt)); + // One level: an entry per node, keyed by its full name, in pre-order (children in name order), + // holding the node's value -- or "" for a node other than this one with neither a value nor + // children. Those are exactly the leaves, in order, of the nested tree this used to build (each + // child's subtree under its full name, a node's value first), and the loader below reads both + // the same way. boost's ptree and its JSON writer and parser recurse once per nesting level, so + // with one level, building this, writing it as JSON, reading it back, copying and destroying it + // no longer recurse once per state level. A heap stack of levels; each node's value and + // children are snapshotted under its own lock, released before going down, and a child's full + // name is carried down as in children(). + struct level { + std::vector> kids; // with their full names + std::size_t next = 0; + }; + const char *const fn = BOOST_CURRENT_FUNCTION; + const auto open = [&pt, fn, this](state &n, const std::string &n_full, bool top) { + level l; + std::string v; + const std::string &base = n._name.empty() ? std::string() : n_full; + { + boost::mutex::scoped_lock lock(n._mutex); + v = n._value; + l.kids.reserve(n._children.size()); + for (const child_map::value_type &kv : n._children) + if (kv.second) + l.kids.emplace_back(kv.second, base.empty() ? kv.first : base + SEPARATOR + kv.first); } + _ctx.log(6, boost::str(boost::format("%s :: %s = %s\n") % fn % n_full % v)); + if (!v.empty()) + pt.push_back(property_tree::ptree::value_type(n_full, property_tree::ptree(v))); + else if (!top && l.kids.empty()) + pt.push_back(property_tree::ptree::value_type(n_full, property_tree::ptree())); + return l; + }; + std::vector stack; + stack.push_back(open(*this, fullName(), true)); + while (!stack.empty()) { + level &l = stack.back(); + if (l.next == l.kids.size()) { + stack.pop_back(); + continue; + } + std::pair kid = std::move(l.kids[l.next++]); + boost::this_thread::interruption_point(); + stack.push_back(open(*kid.first, kid.second, false)); // invalidates `l` } - return pt; } @@ -655,20 +732,31 @@ boost::property_tree::ptree state::ptree() { // 03/16/2012 -- Joe R. -- Creation. namespace { // Deserialize helper. state::ptree() (the serializer above) keys every node by -// its FULL dotted path from the root and NESTS the subtrees, so a node's value -// and its descendants live one or more levels deep. The old loop here was NOT -// recursive: it set only the top-level entries and dropped every nested child, -// so json()/restore()/from_json round-trips silently lost the whole subtree -// (only the shallowest nodes survived). Because every key is a full path from -// the root, a single flat recursive walk restores the tree correctly: for each -// entry that carries a scalar value, set it at its full-path key from `root`. +// its FULL dotted path from the root. It writes one level now, but it used to +// NEST the subtrees, so in documents written before, a node's value and its +// descendants live one or more levels deep. An older loop here set only the +// top-level entries and dropped every nested child, so json()/restore()/from_json +// round-trips silently lost the whole subtree (only the shallowest nodes +// survived). Because every key is a full path from the root, one walk over every +// entry at every level restores either form: for each entry that carries a scalar +// value, set it at its full-path key from `root`. Pre-order, with a heap stack of +// positions rather than a call per nesting level. void apply_state_ptree(cvc::state &root, const boost::property_tree::ptree &pt) { - BOOST_FOREACH (const boost::property_tree::ptree::value_type &v, pt) { + using ptree = boost::property_tree::ptree; + std::vector> stack; + stack.emplace_back(pt.begin(), pt.end()); + while (!stack.empty()) { + std::pair &at = stack.back(); + if (at.first == at.second) { + stack.pop_back(); + continue; + } + const ptree::value_type &v = *at.first++; std::string own = v.second.get_value(); if (!own.empty()) root(v.first).value(own); if (!v.second.empty()) - apply_state_ptree(root, v.second); + stack.emplace_back(v.second.begin(), v.second.end()); // invalidates `at` } } } // namespace @@ -728,12 +816,76 @@ void state::restore(const std::string &filename) { // 03/16/2012 -- Joe R. -- Creation. // 04/15/2012 -- Joe R. -- Triggering enter/exit signals. void state::traverse(traversal_unary_func func, const std::string &re) { + boost::this_thread::interruption_point(); + boost::regex expression; + bool valid = true; + if (!re.empty()) { + try { + expression = boost::regex(re.c_str()); + } catch (boost::bad_expression &) { + _ctx.log( + 2, boost::str(boost::format("%s :: invalid regex '%s'\n") % BOOST_CURRENT_FUNCTION % re)); + valid = false; // as children() does: nothing under this node matches + } + } + + // Each node once, in pre-order (children in name order): this node, then each descendant whose + // full name matches `re` (every one when it is empty). A visit fires the node's traverseEnter, + // calls func with its full name, visits what is under it, then fires its traverseExit, so a + // visited node's enter/exit pair encloses those of the visited nodes below it. A node that + // does not match is not visited, but the nodes under it still are. + // + // This used to traverse() every descendant children() listed, each of which did the same, so + // a node was visited once for every ancestor in the walk (2^(d-1) times d levels down a + // chain), one native call per level. Now a heap stack of levels. A node's children are + // snapshotted under its own lock once func has seen it, as children() was called after func, + // and are held: the walk never finds a node again by name, which recreated one removed + // meanwhile, and each level keeps its node, so a visited node's ancestors outlive its visit. + struct level { + state *node; + state_ptr held; // null for this node: the caller holds it + bool visited; // fire traverseExit when the level closes + std::vector> kids; // with their full names + std::size_t next = 0; + }; + const auto open = [](state &n, const std::string &n_full, state_ptr held, bool visited) { + level l{&n, std::move(held), visited, {}, 0}; + const std::string &base = n._name.empty() ? std::string() : n_full; + boost::mutex::scoped_lock lock(n._mutex); + l.kids.reserve(n._children.size()); + for (const child_map::value_type &kv : n._children) + if (kv.second) + l.kids.emplace_back(kv.second, base.empty() ? kv.first : base + SEPARATOR + kv.first); + return l; + }; + + const std::string full = fullName(); traverseEnter(); - func(fullName()); - std::vector ch = children(re); - BOOST_FOREACH (std::string c, ch) - instance(_ctx)(c).traverse(func, re); - traverseExit(); + func(full); + if (!valid) { + traverseExit(); + return; + } + std::vector stack; + stack.push_back(open(*this, full, state_ptr(), true)); + while (!stack.empty()) { + level &l = stack.back(); + if (l.next == l.kids.size()) { + if (l.visited) + l.node->traverseExit(); + stack.pop_back(); + continue; + } + std::pair kid = std::move(l.kids[l.next++]); + boost::this_thread::interruption_point(); + state &k = *kid.first; + const bool visit = re.empty() || boost::regex_match(kid.second, expression); + if (visit) { + k.traverseEnter(); + func(kid.second); + } + stack.push_back(open(k, kid.second, std::move(kid.first), visit)); // invalidates `l` + } } // ----------- diff --git a/src/cvc/tests/state_deep_tree_test.cpp b/src/cvc/tests/state_deep_tree_test.cpp index 85c1b8c09..c76ea5eb3 100644 --- a/src/cvc/tests/state_deep_tree_test.cpp +++ b/src/cvc/tests/state_deep_tree_test.cpp @@ -1,13 +1,17 @@ // cvc::state on deep trees. A write builds the node's full path (fullName()) and signals every // ancestor (childChanged, the root's slots first); listing descendants (children()) walks the -// subtree; destroying a node destroys its subtree. None of that may recurse once per tree -// level, or a deep tree overflows the native stack. Each body runs on a thread with a small -// fixed stack (kStackKiB, wasm's 64 KiB), so per-level recursion fails here on every platform. +// subtree; destroying a node destroys its subtree; json()/save() and their loaders, traverse() +// and reset() walk it too. None of that may recurse once per tree level, or a deep tree +// overflows the native stack. Each body runs on a thread with a small fixed stack (kStackKiB, +// wasm's 64 KiB), so per-level recursion fails here on every platform. // // For probing, CVC_STATE_DEEP_DEPTH overrides the depth and CVC_STATE_DEEP_STACK_KIB the stack // (0 runs the body on the calling thread). CVC_STATE_BENCH=1 runs the write benchmark. #include +#include +#include +#include #include #include #include @@ -16,8 +20,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -243,6 +249,225 @@ TEST_F(StateDeepTree, WriteThroughALinkSignalsTheTargetsAncestors) { c2.disconnect(); } +// ---- json / save / restore ----------------------------------------------------- + +namespace { +// The values down the chain of "n" nodes under `top`, top first. Creates nothing. +std::vector chain_values(cvc::state &top) { + std::vector out; + for (cvc::state *node = ⊤ node; node = node->findDescendant("n")) + out.push_back(node->value()); + return out; +} + +// The entries of a json() document, in order, as (key, value). An entry that nests fails. +std::vector> json_entries(const std::string &j) { + std::stringstream ss(j); + boost::property_tree::ptree pt; + boost::property_tree::read_json(ss, pt); + std::vector> out; + for (const boost::property_tree::ptree::value_type &kv : pt) { + EXPECT_TRUE(kv.second.empty()) << kv.first << " nests"; + out.emplace_back(kv.first, kv.second.data()); + } + return out; +} +} // namespace + +TEST_F(StateDeepTree, JsonRoundTripsADeepTree) { + // Every node is keyed by its full path, so the document is quadratic in depth, and loading it + // resolves each key from the root: a shorter chain. json() and save() write it, json(j) and + // restore() read it into a fresh app. + const std::size_t n = std::min(depth(), 1000); + cvc::state *node = &root("saved"); + node->value("0"); + for (std::size_t i = 1; i <= n; ++i) { + node = &(*node)("n"); + node->value(std::to_string(i)); + } + const std::vector expected = chain_values(root("saved")); + ASSERT_EQ(expected.size(), n + 1); + const std::string file = + (std::filesystem::temp_directory_path() / "state_deep_tree_save.json").string(); + std::string j; + on_small_stack([&] { + j = root("saved").json(); + root("saved").save(file); + }); + cvc::app from_json, from_file; + cvc::state &r1 = cvc::state::instance(from_json); + cvc::state &r2 = cvc::state::instance(from_file); + on_small_stack([&] { + r1.json(j); + r2.restore(file); + }); + std::error_code ec; + std::filesystem::remove(file, ec); + EXPECT_EQ(chain_values(r1("saved")), expected); + EXPECT_EQ(chain_values(r2("saved")), expected); +} + +TEST_F(StateDeepTree, JsonIsOneLevelOfFullPaths) { + // One entry per node, keyed by its full path, in pre-order (children in name order): the + // node's value, or "" for a node with neither a value nor children. Those are the leaves of + // the nested document json() used to write, which still loads. + root("j").value("1"); + root("j.b.c").value("3"); + root("j.b.e").value("5"); + root("j.d"); + const std::vector> expected{ + {"j", "1"}, {"j.b.c", "3"}, {"j.b.e", "5"}, {"j.d", ""}}; + EXPECT_EQ(json_entries(root("j").json()), expected); + boost::property_tree::ptree pt = root("j").ptree(); + ASSERT_EQ(pt.size(), expected.size()); + for (const boost::property_tree::ptree::value_type &kv : pt) + EXPECT_TRUE(kv.second.empty()) << kv.first << " nests"; + + const std::string nested = + R"({"j": "1", "j.b": {"j.b.c": {"j.b.c": "3"}, "j.b.e": {"j.b.e": "5"}}, "j.d": ""})"; + cvc::app other; + cvc::state &r = cvc::state::instance(other); + r.json(nested); + EXPECT_EQ(r("j").value(), "1"); + EXPECT_EQ(r("j.b.c").value(), "3"); + EXPECT_EQ(r("j.b.e").value(), "5"); + // The loader skips empty values, so j.d is not recreated. + EXPECT_EQ(json_entries(r("j").json()), (std::vector>{ + {"j", "1"}, {"j.b.c", "3"}, {"j.b.e", "5"}})); +} + +// ---- traverse ------------------------------------------------------------------ + +TEST_F(StateDeepTree, TraverseVisitsEachNodeOnce) { + // Pre-order, children in name order, each node once. A node's traverseEnter fires before func + // sees it and its traverseExit after everything under it. With a pattern: the node traverse() + // was called on, then the descendants whose full names match; the walk still goes under the + // ones that do not. + const std::vector paths{"t", "t.b", "t.b.c", "t.b.d", "t.e"}; + std::vector seen; + std::vector conns; + for (const std::string &p : paths) { + const std::string tag = p.substr(p.rfind('.') + 1); + conns.push_back(root(p).traverseEnter.connect([&seen, tag] { seen.push_back("+" + tag); })); + conns.push_back(root(p).traverseExit.connect([&seen, tag] { seen.push_back("-" + tag); })); + } + const auto record = [&seen](std::string name) { seen.push_back(name); }; + root("t").traverse(record); + EXPECT_EQ(seen, (std::vector{"+t", "t", "+b", "t.b", "+c", "t.b.c", "-c", "+d", + "t.b.d", "-d", "-b", "+e", "t.e", "-e", "-t"})); + seen.clear(); + root("t").traverse(record, ".*[ce]"); + EXPECT_EQ(seen, + (std::vector{"+t", "t", "+c", "t.b.c", "-c", "+e", "t.e", "-e", "-t"})); + for (boost::signals2::connection &c : conns) + c.disconnect(); +} + +TEST_F(StateDeepTree, TraverseADeepTree) { + // func gets every node's full name, quadratic in depth by nature: a shorter chain. + const std::size_t n = std::min(depth(), 3000); + cvc::state &top = root("walked"); + chain(top, n); + std::size_t visits = 0, enters = 0, exits = 0; + std::string last; + auto c1 = top.traverseEnter.connect([&] { ++enters; }); + auto c2 = top.traverseExit.connect([&] { ++exits; }); + on_small_stack([&] { + top.traverse([&](std::string name) { + ++visits; + last = std::move(name); + }); + }); + EXPECT_EQ(visits, n + 1); + EXPECT_EQ(last, repeat_path("walked", n)); + EXPECT_EQ(enters, 1u); + EXPECT_EQ(exits, 1u); + c1.disconnect(); + c2.disconnect(); +} + +// ---- reset --------------------------------------------------------------------- + +TEST_F(StateDeepTree, ResetADeepTree) { + cvc::state &top = root("cleared"); + top.value("top"); + cvc::state &leaf = chain(top, depth()); + leaf.value("leaf"); + on_small_stack([&] { top.reset(true, false); }); + EXPECT_EQ(top.value(), ""); + EXPECT_EQ(leaf.value(), ""); + EXPECT_FALSE(leaf.initialized()); + EXPECT_FALSE(leaf.parent()->initialized()); + // reset(true) keeps every node. + EXPECT_EQ(top.findDescendant(repeat_path("n", depth() - 1)), &leaf); +} + +TEST_F(StateDeepTree, ResetADeepTreeSignalsLeafFirst) { + // Each touch signals every ancestor, quadratic in depth: a shorter chain. + const std::size_t n = std::min(depth(), 3000); + cvc::state &top = root("touched"); + chain(top, n).value("leaf"); + std::vector heard; + auto c = root.childChanged.connect([&](const std::string &p) { heard.push_back(p); }); + on_small_stack([&] { top.reset(); }); + c.disconnect(); + ASSERT_EQ(heard.size(), n + 1); + EXPECT_EQ(heard.front(), repeat_path("touched", n)); + EXPECT_EQ(heard.back(), "touched"); +} + +TEST_F(StateDeepTree, ResetClearsTopDownAndTouchesBottomUp) { + // reset(true) clears a node before the nodes under it and touches it after them, a subtree at + // a time: when c (the first leaf) is touched, its later sibling d is not cleared yet. + const std::vector paths{"r", "r.b", "r.b.c", "r.b.d", "r.e"}; + for (const std::string &p : paths) + root(p).value(p); + std::vector seen; + std::vector conns; + for (const std::string &p : paths) + conns.push_back(root(p).valueChanged.connect([&seen, p] { seen.push_back(p); })); + conns.push_back( + root.childChanged.connect([&](const std::string &p) { seen.push_back("^" + p); })); + cvc::state &d = root("r.b.d"); + std::string d_at_c; + conns.push_back(root("r.b.c").valueChanged.connect([&] { d_at_c = d.value(); })); + root("r").reset(); + EXPECT_EQ(seen, (std::vector{"r.b.c", "^r.b.c", "r.b.d", "^r.b.d", "r.b", "^r.b", + "r.e", "^r.e", "r", "^r"})); + EXPECT_EQ(d_at_c, "r.b.d"); + for (boost::signals2::connection &c : conns) + c.disconnect(); +} + +TEST_F(StateDeepTree, ResetSignalsWithoutTheAncestorsLocks) { + // A slot that finds its node from the root (as the sync adapter's do) or reads an ancestor + // takes the ancestors' locks, so reset() must not hold them while it signals. Nor while + // reset(false) destroys the subtree and fires its destroyed() slots. + root("w.b.c").value("c"); + root("w").value("w"); + std::string at_c = "unset"; + auto c1 = root("w.b.c").valueChanged.connect( + [&] { at_c = root("w.b.c").value() + "|" + root("w.b").value() + "|" + root("w").value(); }); + root("z.k").value("k"); + root("z").value("z"); + std::string at_destroy = "unset"; + auto c2 = root("z.k").destroyed.connect([&] { at_destroy = root("z").value(); }); + boost::thread t([&] { + root("w").reset(); + root("z").reset(false); + }); + if (!t.try_join_for(boost::chrono::seconds(60))) { + // Deadlocked on a lock it holds itself; nothing can unblock it. + std::fprintf(stderr, "reset() deadlocked: it signals while holding an ancestor's lock\n"); + std::fflush(stderr); + std::_Exit(1); + } + EXPECT_EQ(at_c, "||"); // its ancestors were cleared first + EXPECT_EQ(at_destroy, ""); + c1.disconnect(); + c2.disconnect(); +} + TEST_F(StateDeepTree, WriteBench) { if (env_size("CVC_STATE_BENCH", 0) != 1) { SUCCEED(); diff --git a/src/cvc/tests/state_snapshot_protocol_test.cpp b/src/cvc/tests/state_snapshot_protocol_test.cpp index 42b9604c2..89f7cddd6 100644 --- a/src/cvc/tests/state_snapshot_protocol_test.cpp +++ b/src/cvc/tests/state_snapshot_protocol_test.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -87,6 +88,21 @@ TEST_F(StateSnapshotProtocolTest, SnapshotWithPrefixFilter) { shard.detach(); } +TEST_F(StateSnapshotProtocolTest, SnapshotListsEachNodeOnce) { + // The snapshot walks the subtree with state::traverse(), which visits each node once. It used + // to visit a node once per ancestor in the walk, so a.b.c was listed (and encoded) twice. + cvc::app ctx; + cvc::state_cluster_shard shard(ctx, "clust", "node_A"); + shard.attach(); + cvc::state::instance(ctx)("a.b.c").value(std::string("deep")); + cvc::state::instance(ctx)("a.d").value(std::string("x")); + std::map seen; + for (const auto &e : shard.snapshot("a")) + ++seen[e.path]; + EXPECT_EQ(seen, (std::map{{"a", 1}, {"a.b", 1}, {"a.b.c", 1}, {"a.d", 1}})); + shard.detach(); +} + TEST_F(StateSnapshotProtocolTest, IpcSnapshotRequestResponse) { std::string sock = make_socket_path("snap_ipc"); sockets.push_back(sock);