Merge #14985: test: Remove thread_local from test_bitcoin

fa61202cae test: Add comment to g_insecure_rand_ctx (MarcoFalke)
fa0d3c4407 test: Undo thread_local g_insecure_rand_ctx (MarcoFalke)

Pull request description:

  `thread_local` seems to be highly controversial according to the discussion in #14953, so remove it again from the tests.

  Also remove boost::thread_group in the test that uses it, since I am touching it anyway.

Tree-SHA512: 977c1f597e3cfbd0e97d0b037d998fdbc701f62e9a2f57e02dbe1727b63ae8ff478dbd9d3d6dc4ffdfa23f2058b331f04949d51f23a8f55b41ecb75f088f1cbe

3 files changed, 17 insertions, 7 deletions

diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp
index bb8db9fa8d..a3201de385 100644
--- a/src/test/test_bitcoin.cpp
+++ b/src/test/test_bitcoin.cpp
@@ -22,6 +22,8 @@
 
 const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
 
+FastRandomContext g_insecure_rand_ctx;
+
 void CConnmanTest::AddNode(CNode& node)
 {
     LOCK(g_connman->cs_vNodes);
@@ -37,8 +39,6 @@ void CConnmanTest::ClearNodes()
     g_connman->vNodes.clear();
 }
 
-thread_local FastRandomContext g_insecure_rand_ctx;
-
 std::ostream& operator<<(std::ostream& os, const uint256& num)
 {
     os << num.ToString();
diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h
index 71c0379eac..31d90c0151 100644
--- a/src/test/test_bitcoin.h
+++ b/src/test/test_bitcoin.h
@@ -26,7 +26,14 @@ std::ostream& operator<<(typename std::enable_if<std::is_enum<T>::value, std::os
     return stream << static_cast<typename std::underlying_type<T>::type>(e);
 }
 
-thread_local extern FastRandomContext g_insecure_rand_ctx;
+/**
+ * This global and the helpers that use it are not thread-safe.
+ *
+ * If thread-safety is needed, the global could be made thread_local (given
+ * that thread_local is supported on all architectures we support) or a
+ * per-thread instance could be used in the multi-threaded test.
+ */
+extern FastRandomContext g_insecure_rand_ctx;
 
 static inline void SeedInsecureRand(bool deterministic = false)
 {
diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
index 7dd176b25d..62dfed6c41 100644
--- a/src/test/validation_block_tests.cpp
+++ b/src/test/validation_block_tests.cpp
@@ -152,12 +152,13 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
     // create a bunch of threads that repeatedly process a block generated above at random
     // this will create parallelism and randomness inside validation - the ValidationInterface
     // will subscribe to events generated during block validation and assert on ordering invariance
-    boost::thread_group threads;
+    std::vector<std::thread> threads;
     for (int i = 0; i < 10; i++) {
-        threads.create_thread([&blocks]() {
+        threads.emplace_back([&blocks]() {
             bool ignored;
+            FastRandomContext insecure;
             for (int i = 0; i < 1000; i++) {
-                auto block = blocks[InsecureRandRange(blocks.size() - 1)];
+                auto block = blocks[insecure.randrange(blocks.size() - 1)];
                 ProcessNewBlock(Params(), block, true, &ignored);
             }
 
@@ -171,7 +172,9 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering)
         });
     }
 
-    threads.join_all();
+    for (auto& t : threads) {
+        t.join();
+    }
     while (GetMainSignals().CallbacksPending() > 0) {
         MilliSleep(100);
     }