From 59b87a27efea819e433c727756bf5fac57b33dd6 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 7 May 2018 17:08:03 -0400 Subject: [PATCH 1/7] [wallet] Fix potential memory leak in CreateWalletFromFile Fix proposed by ryanofsky in https://github.com/bitcoin/bitcoin/pull/12647#discussion_r174875670 --- src/wallet/wallet.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5be9f4a2906..f9f567009ac 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4012,7 +4012,10 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& int64_t nStart = GetTimeMillis(); bool fFirstRun = true; - CWallet *walletInstance = new CWallet(name, WalletDatabase::Create(path)); + // Make a temporary wallet unique pointer so memory doesn't get leaked if + // wallet creation fails. + auto temp_wallet = MakeUnique(name, WalletDatabase::Create(path)); + CWallet* walletInstance = temp_wallet.get(); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DBErrors::LOAD_OK) { @@ -4224,7 +4227,6 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& } walletInstance->m_last_block_processed = chainActive.Tip(); - RegisterValidationInterface(walletInstance); if (chainActive.Tip() && chainActive.Tip() != pindexRescan) { @@ -4290,6 +4292,10 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& } } } + + // Register with the validation interface. It's ok to do this after rescan since we're still holding cs_main. + RegisterValidationInterface(temp_wallet.release()); + walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); { From 470316c3bf5ca343d5d66b94839169a4572eceb7 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 19 Apr 2018 17:42:40 -0400 Subject: [PATCH 2/7] [wallet] setup wallet background flushing in WalletInit directly WalletInit::Start calls postInitProcess() for each wallet. Previously each call to postInitProcess() would attempt to schedule wallet background flushing. Just start wallet background flushing once from WalletInit::Start(). --- src/wallet/init.cpp | 6 +++++- src/wallet/wallet.cpp | 10 +--------- src/wallet/wallet.h | 4 +--- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 6c5522e4bc0..e9710012b5f 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -264,8 +265,11 @@ bool WalletInit::Open() const void WalletInit::Start(CScheduler& scheduler) const { for (CWallet* pwallet : GetWallets()) { - pwallet->postInitProcess(scheduler); + pwallet->postInitProcess(); } + + // Run a thread to flush wallet periodically + scheduler.scheduleEvery(MaybeCompactWalletDB, 500); } void WalletInit::Flush() const diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f9f567009ac..4d3e3813afe 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -23,7 +23,6 @@ #include #include #include