diff options
author | MarcoFalke <falke.marco@gmail.com> | 2018-12-19 07:40:37 +1300 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2018-12-19 07:40:53 +1300 |
commit | d4197812d47d2452b081b36ac06a032765f9f5a7 (patch) | |
tree | bbcda96ddf56f7a67e1821bfeb198db850528dbf | |
parent | f055389cb9572ed96c93722f451dfb37e7f1f9a2 (diff) | |
parent | fa61202cae6979c754b11842c5ed8d90f1cdcc45 (diff) | |
download | bitcoin-d4197812d47d2452b081b36ac06a032765f9f5a7.tar.xz |
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
-rw-r--r-- | src/test/test_bitcoin.cpp | 4 | ||||
-rw-r--r-- | src/test/test_bitcoin.h | 9 | ||||
-rw-r--r-- | src/test/validation_block_tests.cpp | 11 |
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); } |