coins: add ready flag to InputToFetch
What changed, and why it matters
This is a preparatory code change for future multi-threaded coin fetching in Bitcoin Core. It adds a synchronization flag (std::atomic_flag) around a shared data field so that worker threads can safely signal when they have finished writing a 'coin' object and the main thread can safely read it. The commit itself does not introduce a known exploitable vulnerability; it is a defensive concurrency fix to prevent potential race conditions or memory-visibility bugs in upcoming parallel code.
Review as a normal correctness/concurrency hardening change. No immediate security response required. Monitor follow-up commits that actually enable multi-threaded ProcessInput to ensure the synchronization pattern is complete and correct.
Security signals we found
Concurrency synchronization added for shared coin field
Use of release/acquire atomic semantics to prevent data races
Preparation for multi-threaded ProcessInput execution
Assertions guard against moving already-initialized/ready state
Evidence from the diff
The patch modifies src/coins.h to add std::atomic_flag ready to the InputToFetch struct and uses release/acquire memory ordering with wait/notify to synchronize writes and reads of the coin field across threads. A move constructor is added with assertions that the source is not already ready and has no coin. The worker thread sets the flag with release semantics after writing coin; the main thread waits on the flag with acquire semantics before reading/moving coin. This is a correctness fix for a future multi-threaded ProcessInput path.
Changed components
src/coins.hCCoinsViewCache::InputToFetchCCoinsViewCache::StartFetching / FetchCoin / ProcessInputInspect captured patch +15 / −0
diff --git a/src/coins.h b/src/coins.h
index 23c5fa5f..df519f76 100644
--- a/src/coins.h
+++ b/src/coins.h
@@ -580,6 +580,8 @@ private:
//! The inputs of the block which is being fetched.
struct InputToFetch {
+ //! Workers set this after setting the coin. The main thread tests this before reading the coin.
+ std::atomic_flag ready{};
//! The outpoint of the input to fetch.
const COutPoint& outpoint;
//! The coin that workers will fetch and main thread will insert into cache.
@@ -587,6 +589,14 @@ private:
mutable std::optional<Coin> coin{std::nullopt};
explicit InputToFetch(const COutPoint& o LIFETIMEBOUND) noexcept : outpoint{o} {}
+
+ //! Move ctor is required for resizing m_inputs in StartFetching. Elements will never move once parallel tasks
+ //! are started, so we can assert that coin is nullopt and ready is false.
+ InputToFetch(InputToFetch&& other) noexcept : outpoint{other.outpoint}
+ {
+ Assert(!other.coin);
+ Assert(!other.ready.test(std::memory_order_relaxed));
+ }
};
std::vector<InputToFetch> m_inputs{};
@@ -603,6 +613,9 @@ private:
auto& input{m_inputs[i]};
input.coin = base->PeekCoin(input.outpoint);
+ // Use release so writing coin above happens before the main thread acquires.
+ Assert(!input.ready.test_and_set(std::memory_order_release));
+ input.ready.notify_one();
return true;
}
@@ -621,6 +634,8 @@ private:
if (m_input_tail < m_inputs.size() && m_inputs[m_input_tail].outpoint == outpoint) {
// We advance the tail since the input is cached and not accessed through this method again.
auto& input{m_inputs[m_input_tail++]};
+ // Wait until the coin is ready to be read. We need acquire so we match the worker thread's release.
+ input.ready.wait(/*old=*/false, std::memory_order_acquire);
// We can move the coin since we won't access this input again.
return std::move(input.coin);
}
Why this scored 29/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.