Merge bitcoin#18615: test: Avoid accessing free'd memory in validatio…

…n_chainstatemanager_tests

fa176e2 test: Avoid accessing free'd memory in validation_chainstatemanager_tests (MarcoFalke)

Pull request description:

ACKs for top commit:
  ryanofsky:
    Code review ACK fa176e2, though if you have to update this again, would suggest separating txindex test cleanup and the chainstatemanager test fix in separate commits, or identifying which part of the change is the bugfix fix in the commit description. Also to clean up the txindex test it might make sense to call SyncWithValidationInterfaceQueue in the test destructor to prevent nondeterminism in other tests

Tree-SHA512: 34c5dca283a7c205cd42b6aa59f00a71fd1bd980bc3d6640a18b280be11470bfabb2fd8c93fadde6fb8e084bcf96c80ec3aa72bbccccfde8a8260d173eaad08f

src/test/txindex_tests.cpp

@@ -72,10 +72,8 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup)
     // shutdown sequence (c.f. Shutdown() in init.cpp)
     txindex.Stop();
 
-    threadGroup.interrupt_all();
-    threadGroup.join_all();
-
-    // Rest of shutdown sequence and destructors happen in ~TestingSetup()
+    // Let scheduler events finish running to avoid accessing any memory related to txindex after it is destructed
+    SyncWithValidationInterfaceQueue();
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/test/validation_chainstatemanager_tests.cpp

@@ -0,0 +1,107 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+//
+#include <chainparams.h>
+#include <consensus/validation.h>
+#include <random.h>
+#include <sync.h>
+#include <test/util/setup_common.h>
+#include <uint256.h>
+#include <validation.h>
+#include <validationinterface.h>
+
+#include <vector>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(validation_chainstatemanager_tests, TestingSetup)
+
+//! Basic tests for ChainstateManager.
+//!
+//! First create a legacy (IBD) chainstate, then create a snapshot chainstate.
+BOOST_AUTO_TEST_CASE(chainstatemanager)
+{
+    ChainstateManager manager;
+    std::vector<CChainState*> chainstates;
+    const CChainParams& chainparams = Params();
+
+    // Create a legacy (IBD) chainstate.
+    //
+    ENTER_CRITICAL_SECTION(cs_main);
+    CChainState& c1 = manager.InitializeChainstate();
+    LEAVE_CRITICAL_SECTION(cs_main);
+    chainstates.push_back(&c1);
+    c1.InitCoinsDB(
+        /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
+    WITH_LOCK(::cs_main, c1.InitCoinsCache());
+
+    BOOST_CHECK(!manager.IsSnapshotActive());
+    BOOST_CHECK(!manager.IsSnapshotValidated());
+    BOOST_CHECK(!manager.IsBackgroundIBD(&c1));
+    auto all = manager.GetAll();
+    BOOST_CHECK_EQUAL_COLLECTIONS(all.begin(), all.end(), chainstates.begin(), chainstates.end());
+
+    auto& active_chain = manager.ActiveChain();
+    BOOST_CHECK_EQUAL(&active_chain, &c1.m_chain);
+
+    BOOST_CHECK_EQUAL(manager.ActiveHeight(), -1);
+
+    auto active_tip = manager.ActiveTip();
+    auto exp_tip = c1.m_chain.Tip();
+    BOOST_CHECK_EQUAL(active_tip, exp_tip);
+
+    auto& validated_cs = manager.ValidatedChainstate();
+    BOOST_CHECK_EQUAL(&validated_cs, &c1);
+
+    // Create a snapshot-based chainstate.
+    //
+    ENTER_CRITICAL_SECTION(cs_main);
+    CChainState& c2 = manager.InitializeChainstate(GetRandHash());
+    LEAVE_CRITICAL_SECTION(cs_main);
+    chainstates.push_back(&c2);
+    c2.InitCoinsDB(
+        /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
+    WITH_LOCK(::cs_main, c2.InitCoinsCache());
+    // Unlike c1, which doesn't have any blocks. Gets us different tip, height.
+    c2.LoadGenesisBlock(chainparams);
+    BlockValidationState _;
+    BOOST_CHECK(c2.ActivateBestChain(_, chainparams, nullptr));
+
+    BOOST_CHECK(manager.IsSnapshotActive());
+    BOOST_CHECK(!manager.IsSnapshotValidated());
+    BOOST_CHECK(manager.IsBackgroundIBD(&c1));
+    BOOST_CHECK(!manager.IsBackgroundIBD(&c2));
+    auto all2 = manager.GetAll();
+    BOOST_CHECK_EQUAL_COLLECTIONS(all2.begin(), all2.end(), chainstates.begin(), chainstates.end());
+
+    auto& active_chain2 = manager.ActiveChain();
+    BOOST_CHECK_EQUAL(&active_chain2, &c2.m_chain);
+
+    BOOST_CHECK_EQUAL(manager.ActiveHeight(), 0);
+
+    auto active_tip2 = manager.ActiveTip();
+    auto exp_tip2 = c2.m_chain.Tip();
+    BOOST_CHECK_EQUAL(active_tip2, exp_tip2);
+
+    // Ensure that these pointers actually correspond to different
+    // CCoinsViewCache instances.
+    BOOST_CHECK(exp_tip != exp_tip2);
+
+    auto& validated_cs2 = manager.ValidatedChainstate();
+    BOOST_CHECK_EQUAL(&validated_cs2, &c1);
+
+    auto& validated_chain = manager.ValidatedChain();
+    BOOST_CHECK_EQUAL(&validated_chain, &c1.m_chain);
+
+    auto validated_tip = manager.ValidatedTip();
+    exp_tip = c1.m_chain.Tip();
+    BOOST_CHECK_EQUAL(validated_tip, exp_tip);
+
+    // Let scheduler events finish running to avoid accessing memory that is going to be unloaded
+    SyncWithValidationInterfaceQueue();
+
+    WITH_LOCK(::cs_main, manager.Unload());
+}
+
+BOOST_AUTO_TEST_SUITE_END()