Merge bitcoin/bitcoin#29243: wallet: Reset chain notifications handler if AttachChain fails

ea2551e55d wallet: Reset chain notifications handler if AttachChain fails (Ava Chow)

Pull request description:

  AttachChain will create the chain notifications handler which contains a reference to the wallet's shared_ptr. If AttachChain fails, the wallet needs to be unloaded, and this is expected to happen with its custom deleter ReleaseWallet. However, if the chain notifications handler is still set, then the shared_ptr is still referenced by something, so the wallet is never actually released.

  This behavior can also be verified by looking at the debug.log file. When the wallet is released, the line "Releasing wallet" should appear in the debug.log file. However the failing test does not contain that line, indicating that the problem is that the `CWallet` object is not being destroyed. After this PR, that log line now appears, and the test also passes.

  Fixes #29234

ACKs for top commit:
  jamesob:
    ACK ea2551e55d
  murchandamus:
    ACK ea2551e55d
  TheCharlatan:
    ACK ea2551e55d
  furszy:
    Code review ACK ea2551e5
  BrandonOdiwuor:
    Code Review ACK ea2551e55d

Tree-SHA512: 73d676068c699303d9bcf70c9288ecb05f5f2e400ff3b7201367658d39d2fab63def97ab5ce4a742a6f2ca5e325f598fdbb6ce8f157a0423c07abc9a19bd5c81
This commit is contained in:
fanquake 2024-01-15 10:02:47 +00:00
commit fe1eccd4d7
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1

View File

@ -3099,6 +3099,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
if (time_first_key) walletInstance->MaybeUpdateBirthTime(*time_first_key);
if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) {
walletInstance->m_chain_notifications_handler.reset(); // Reset this pointer so that the wallet will actually be unloaded
return nullptr;
}