Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#28224: shutdown: Destroy kernel last, make test…
Browse files Browse the repository at this point in the history
… shutdown order consistent

c1144f0 tests: Reset node context members on ~BasicTestingSetup (TheCharlatan)
9759af1 shutdown: Destroy kernel last (TheCharlatan)

Pull request description:

  The destruction/resetting of node context members in the tests should roughly follow the behavior of the `Shutdown` function in `init.cpp`.

  This was originally requested by MarcoFalke in this [comment](bitcoin/bitcoin#25065 (comment)) in response to the [original pull request](bitcoin/bitcoin#25065) introducing the `kernel::Context`.

ACKs for top commit:
  maflcko:
    ACK c1144f0 🗣
  achow101:
    ACK c1144f0
  ryanofsky:
    Code review ACK c1144f0. No code changes since last review, just updated commits and descriptions

Tree-SHA512: 819bb85ff82a5c6c60e429674d5684f3692fe9062500d00a87b361cc59e6bda145be21b5a4466dee6791faed910cbde4d26baab325bf6daa1813af13a63588ff
  • Loading branch information
achow101 committed Nov 7, 2023
2 parents c8a883a + c1144f0 commit c981771
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,11 +343,11 @@ void Shutdown(NodeContext& node)
node.chain_clients.clear();
UnregisterAllValidationInterfaces();
GetMainSignals().UnregisterBackgroundSignalScheduler();
node.kernel.reset();
node.mempool.reset();
node.fee_estimator.reset();
node.chainman.reset();
node.scheduler.reset();
node.kernel.reset();

try {
if (!fs::remove(GetPidFile(*node.args))) {
Expand Down
4 changes: 3 additions & 1 deletion src/test/util/setup_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto

BasicTestingSetup::~BasicTestingSetup()
{
m_node.kernel.reset();
SetMockTime(0s); // Reset mocktime for following tests
LogInstance().DisconnectTestLogger();
fs::remove_all(m_path_root);
Expand Down Expand Up @@ -205,8 +206,9 @@ ChainTestingSetup::~ChainTestingSetup()
m_node.netgroupman.reset();
m_node.args = nullptr;
m_node.mempool.reset();
m_node.scheduler.reset();
m_node.fee_estimator.reset();
m_node.chainman.reset();
m_node.scheduler.reset();
}

void ChainTestingSetup::LoadVerifyActivateChainstate()
Expand Down

0 comments on commit c981771

Please sign in to comment.