highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-17 01:58:57 +02:00
Code Issues Packages Projects Releases Wiki Activity

test: Check btcsignals determinism in thread_safety test case

Browse Source 
The test only checked that the single atomic value is greater than 3000.
However, by splitting the atomic into two, one can do one exact check,
and also increase the lower bound on the inexact check.

Also, test disconnect races for every second step, instead of only once
at the end (likely when only one thread is running anyway).

Both changes make the test stricter and may catch non-determinism issues
that are not detected by sanitizers alone.

The test added in this commit should also pass when applied on top of
commit 63c68e2a3f, which is still using
the boost implementation.
This commit is contained in:
MarcoFalke
2026-04-09 13:01:51 +02:00
parent fa86e5dba9
commit fa1bc1fe51
1 changed files with 22 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
src/test/btcsignals_tests.cpp
View File

@@ -111,17 +111,18 @@ BOOST_AUTO_TEST_CASE(any_of_combiner)
* Connect sig0 to an incrementor function and loop in a thread.
* Meanwhile, in another thread, inject and call new increment callbacks.
* Both threads are constantly calling empty/connected.
* Though the end-result is undefined due to a non-deterministic number of
* total callbacks executed, this should all be completely threadsafe.
* The end-result must be deterministic for the atomic modified by conn0.
* Though, the end-result for the atomic modified by the extra connections is
* undefined due to a non-deterministic number of total callbacks executed.
* In any case, this should all be completely threadsafe.
* Sanitizers should pick up any buggy data race behavior (if present).
*/
BOOST_AUTO_TEST_CASE(thread_safety)
{
btcsignals::signal<void()> sig0;
std::atomic<uint32_t> val{0};
auto conn0 = sig0.connect([&val] {
val++;
});
std::atomic<uint32_t> val_det{0};
std::atomic<uint32_t> val_non_det{0};
auto conn0 = sig0.connect([&val_det] { val_det++; });
std::thread incrementor([&conn0, &sig0] {
for (int i = 0; i < 1000; i++) {
@@ -134,16 +135,17 @@ BOOST_AUTO_TEST_CASE(thread_safety)
assert(conn0.connected());
});
std::thread extra_increment_injector([&conn0, &sig0, &val] {
std::thread extra_increment_injector([&conn0, &sig0, &val_non_det] {
static constexpr size_t num_extra_conns{1000};
std::vector<btcsignals::scoped_connection> extra_conns;
extra_conns.reserve(num_extra_conns);
for (size_t i = 0; i < num_extra_conns; i++) {
BOOST_CHECK(!sig0.empty());
BOOST_CHECK(conn0.connected());
extra_conns.emplace_back(sig0.connect([&val] {
val++;
}));
btcsignals::scoped_connection extra{sig0.connect([&val_non_det] { val_non_det++; })};
if (i % 2 == 0) {
extra_conns.emplace_back(std::move(extra));
}
sig0();
}
});
@@ -152,10 +154,16 @@ BOOST_AUTO_TEST_CASE(thread_safety)
conn0.disconnect();
BOOST_CHECK(sig0.empty());
// sig will have been called 2000 times, and at least 1000 of those will
// have been executing multiple incrementing callbacks. So while val is
// probably MUCH bigger, it's guaranteed to be at least 3000.
BOOST_CHECK_GE(val.load(), 3000);
// sig0 will have been called 2000 times, and only the first connection did
// increment val_det, so it must be 2000.
BOOST_CHECK_EQUAL(val_det.load(), 2000);
// The number of connections that increment val_non_det is growing from 1
// to 500, where 500 are disconnected immediately again after the step.
// Before the end of each step the connections are called at least once.
// However, it is unknown how often the connections have been called
// exactly. The 500th Triangular Number gives a lower estimate.
// T_n=n(n+1)/2
BOOST_CHECK_GE(val_non_det.load(), 500 * 501 / 2);
}
/* Test that connection and disconnection works from within signal