0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00

Merge #17897: init: Stop indexes on shutdown after ChainStateFlushed callback.

9dd58ca611 init: Stop indexes on shutdown after ChainStateFlushed callback. (Jim Posen)

Pull request description:

  Replaces https://github.com/bitcoin/bitcoin/pull/17852.

  Currently, the latest index state may not be committed to disk on shutdown. The state is committed on `ChainStateFlushed` callbacks and the current init order unregisters the indexes as validation interfaces before the final `ChainStateFlushed` callback is called on them.

  Issue identified by paulyc.

  For review: an alternative or supplemental solution would be to call `Commit` at the end of `BaseIndex::Stop`. I don't see any harm in doing so and it makes the less prone to user error. However, the destructor would have to be modified to not call `Stop` because `Commit` calls a virtual method, so I figured it wasn't worth it. But I'm curious how others feel.

ACKs for top commit:
  fjahr:
    tested ACK 9dd58ca611
  paulyc:
    > Code review ACK [9dd58ca](9dd58ca611), but failed to test because I can't reproduce the original problem.
  kallewoof:
    Tested ACK 9dd58ca611
  promag:
    Code review ACK 9dd58ca611, but failed to test because I can't reproduce the original problem.

Tree-SHA512: 2918380b699833cb7eab07456d1667dbf8ebbe2d2b5988300a3cf5b6a6cfc818b6d9086e1936ffe7881f67e409306c4b91d61a08a169cfd0a301383479d4f3cb
This commit is contained in:
fanquake 2020-01-22 17:30:00 +08:00
commit a51aa2880d
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1

View file

@ -197,8 +197,6 @@ void Shutdown(NodeContext& node)
// using the other before destroying them.
if (node.peer_logic) UnregisterValidationInterface(node.peer_logic.get());
if (node.connman) node.connman->Stop();
if (g_txindex) g_txindex->Stop();
ForEachBlockFilterIndex([](BlockFilterIndex& index) { index.Stop(); });
StopTorControl();
@ -212,8 +210,6 @@ void Shutdown(NodeContext& node)
node.peer_logic.reset();
node.connman.reset();
node.banman.reset();
g_txindex.reset();
DestroyAllBlockFilterIndexes();
if (::mempool.IsLoaded() && gArgs.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
DumpMempool(::mempool);
@ -246,6 +242,14 @@ void Shutdown(NodeContext& node)
// CValidationInterface callbacks, flush them...
GetMainSignals().FlushBackgroundCallbacks();
// Stop and delete all indexes only after flushing background callbacks.
if (g_txindex) {
g_txindex->Stop();
g_txindex.reset();
}
ForEachBlockFilterIndex([](BlockFilterIndex& index) { index.Stop(); });
DestroyAllBlockFilterIndexes();
// Any future callbacks will be dropped. This should absolutely be safe - if
// missing a callback results in an unrecoverable situation, unclean shutdown
// would too. The only reason to do the above flushes is to let the wallet catch