Skip to content

Commit

Permalink
Merge bitcoin#30896: kernel: Move background load thread to node context
Browse files Browse the repository at this point in the history
bc7900f kernel: Move background load thread to node context (TheCharlatan)

Pull request description:

  The thread handle is never used by the ChainstateManager, so move it out and into the node context. Users of the kernel library now no longer have to manually join the thread when destructing the ChainstateManager.

ACKs for top commit:
  maflcko:
    ACK bc7900f 🔄
  achow101:
    ACK bc7900f
  ryanofsky:
    Code review ACK bc7900f. Nice cleanup
  jonatack:
    Light ACK bc7900f
  stickies-v:
    ACK bc7900f

Tree-SHA512: add9c4823731324e3db50f95e023e99d55db7cc75c69083ae7c9c2157e5540968caa6cf10674aa4901f91366b02ebb1ff18bb977fec0a46431e2196448958b9d
  • Loading branch information
achow101 committed Sep 13, 2024
2 parents 87d5450 + bc7900f commit 0c4ff18
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 6 deletions.
2 changes: 0 additions & 2 deletions src/bitcoin-chainstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,6 @@ int main(int argc, char* argv[])
epilogue:
// Without this precise shutdown sequence, there will be a lot of nullptr
// dereferencing and UB.
if (chainman.m_thread_load.joinable()) chainman.m_thread_load.join();

validation_signals.FlushBackgroundCallbacks();
{
LOCK(cs_main);
Expand Down
4 changes: 2 additions & 2 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ void Shutdown(NodeContext& node)

StopTorControl();

if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join();
if (node.background_init_thread.joinable()) node.background_init_thread.join();
// After everything has been shut down, but before things get flushed, stop the
// the scheduler. After this point, SyncWithValidationInterfaceQueue() should not be called anymore
// as this would prevent the shutdown from completing.
Expand Down Expand Up @@ -1789,7 +1789,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
vImportFiles.push_back(fs::PathFromString(strFile));
}

chainman.m_thread_load = std::thread(&util::TraceThread, "initload", [=, &chainman, &args, &node] {
node.background_init_thread = std::thread(&util::TraceThread, "initload", [=, &chainman, &args, &node] {
ScheduleBatchPriority();
// Import blocks
ImportBlocks(chainman, vImportFiles);
Expand Down
2 changes: 2 additions & 0 deletions src/node/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <cstdlib>
#include <functional>
#include <memory>
#include <thread>
#include <vector>

class ArgsManager;
Expand Down Expand Up @@ -86,6 +87,7 @@ struct NodeContext {
std::atomic<int> exit_status{EXIT_SUCCESS};
//! Manages all the node warnings
std::unique_ptr<node::Warnings> warnings;
std::thread background_init_thread;

//! Declare default constructor and destructor that are not inline, so code
//! instantiating the NodeContext struct doesn't need to #include class
Expand Down
2 changes: 0 additions & 2 deletions src/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
#include <span>
#include <stdint.h>
#include <string>
#include <thread>
#include <type_traits>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -1008,7 +1007,6 @@ class ChainstateManager

const util::SignalInterrupt& m_interrupt;
const Options m_options;
std::thread m_thread_load;
//! A single BlockManager instance is shared across each constructed
//! chainstate to avoid duplicating block metadata.
node::BlockManager m_blockman;
Expand Down

0 comments on commit 0c4ff18

Please sign in to comment.