walletdb: Move BerkeleyDatabase::Flush(true) to Close()

achow101 · achow101 · commit 27b276638499 · 2020-07-14T11:07:16.000-04:00
Instead of having Flush optionally shutdown the database and
environment, add a Close() function that does that.
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
@@ -324,6 +324,15 @@ void BerkeleyEnvironment::CheckpointLSN(const std::string& strFile)
     dbenv->lsn_reset(strFile.c_str(), 0);
 }
 
+BerkeleyDatabase::~BerkeleyDatabase()
+{
+    if (env) {
+        LOCK(cs_db);
+        size_t erased = env->m_databases.erase(strFile);
+        assert(erased == 1);
+        env->m_fileids.erase(strFile);
+    }
+}
 
 BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bool fFlushOnCloseIn) : pdb(nullptr), activeTxn(nullptr), m_cursor(nullptr)
 {
@@ -685,22 +694,17 @@ bool BerkeleyDatabase::Backup(const std::string& strDest) const
     }
 }
 
-void BerkeleyDatabase::Flush(bool shutdown)
+void BerkeleyDatabase::Flush()
 {
     if (!IsDummy()) {
-        env->Flush(shutdown);
-        if (shutdown) {
-            LOCK(cs_db);
-            g_dbenvs.erase(env->Directory().string());
-            env = nullptr;
-        } else {
-            // TODO: To avoid g_dbenvs.erase erasing the environment prematurely after the
-            // first database shutdown when multiple databases are open in the same
-            // environment, should replace raw database `env` pointers with shared or weak
-            // pointers, or else separate the database and environment shutdowns so
-            // environments can be shut down after databases.
-            env->m_fileids.erase(strFile);
-        }
+        env->Flush(false);
+    }
+}
+
+void BerkeleyDatabase::Close()
+{
+    if (!IsDummy()) {
+        env->Flush(true);
     }
 }
 
diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h
@@ -115,12 +115,7 @@ class BerkeleyDatabase
         assert(inserted.second);
     }
 
-    ~BerkeleyDatabase() {
-        if (env) {
-            size_t erased = env->m_databases.erase(strFile);
-            assert(erased == 1);
-        }
-    }
+    ~BerkeleyDatabase();
 
     /** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero
      */
@@ -130,9 +125,13 @@ class BerkeleyDatabase
      */
     bool Backup(const std::string& strDest) const;
 
-    /** Make sure all changes are flushed to disk.
+    /** Make sure all changes are flushed to database file.
+     */
+    void Flush();
+    /** Flush to the database file and close the database.
+     *  Also close the environment if no other databases are open in it.
      */
-    void Flush(bool shutdown);
+    void Close();
     /* flush the wallet passively (TRY_LOCK)
        ideal to be called periodically */
     bool PeriodicFlush();
diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp
@@ -99,14 +99,14 @@ void StartWallets(CScheduler& scheduler, const ArgsManager& args)
 void FlushWallets()
 {
     for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
-        pwallet->Flush(false);
+        pwallet->Flush();
     }
 }
 
 void StopWallets()
 {
     for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
-        pwallet->Flush(true);
+        pwallet->Close();
     }
 }
 
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -439,9 +439,14 @@ bool CWallet::HasWalletSpend(const uint256& txid) const
     return (iter != mapTxSpends.end() && iter->first.hash == txid);
 }
 
-void CWallet::Flush(bool shutdown)
+void CWallet::Flush()
 {
-    database->Flush(shutdown);
+    database->Flush();
+}
+
+void CWallet::Close()
+{
+    database->Close();
 }
 
 void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> range)
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -1087,7 +1087,10 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     bool HasWalletSpend(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     //! Flush wallet (bitdb flush)
-    void Flush(bool shutdown=false);
+    void Flush();
+
+    //! Close wallet database
+    void Close();
 
     /** Wallet is about to be unloaded */
     boost::signals2::signal<void ()> NotifyUnload;
diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp
@@ -17,7 +17,7 @@ namespace WalletTool {
 static void WalletToolReleaseWallet(CWallet* wallet)
 {
     wallet->WalletLogPrintf("Releasing wallet\n");
-    wallet->Flush(true);
+    wallet->Close();
     delete wallet;
 }
 
@@ -133,7 +133,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)
         std::shared_ptr<CWallet> wallet_instance = CreateWallet(name, path);
         if (wallet_instance) {
             WalletShowInfo(wallet_instance.get());
-            wallet_instance->Flush(true);
+            wallet_instance->Close();
         }
     } else if (command == "info" || command == "salvage") {
         if (!fs::exists(path)) {
@@ -145,7 +145,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)
             std::shared_ptr<CWallet> wallet_instance = LoadWallet(name, path);
             if (!wallet_instance) return false;
             WalletShowInfo(wallet_instance.get());
-            wallet_instance->Flush(true);
+            wallet_instance->Close();
         } else if (command == "salvage") {
             return SalvageWallet(path);
         }

Original file line number	Diff line number	Diff line change
`@@ -99,14 +99,14 @@ void StartWallets(CScheduler& scheduler, const ArgsManager& args)`
`99`	`99`	`void FlushWallets()`
`100`	`100`	`{`
`101`	`101`	`for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {`
`102`		`- pwallet->Flush(false);`
	`102`	`+ pwallet->Flush();`
`103`	`103`	`}`
`104`	`104`	`}`
`105`	`105`
`106`	`106`	`void StopWallets()`
`107`	`107`	`{`
`108`	`108`	`for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {`
`109`		`- pwallet->Flush(true);`
	`109`	`+ pwallet->Close();`
`110`	`110`	`}`
`111`	`111`	`}`
`112`	`112`
Original file line number	Diff line number	Diff line change
`@@ -439,9 +439,14 @@ bool CWallet::HasWalletSpend(const uint256& txid) const`
`439`	`439`	`return (iter != mapTxSpends.end() && iter->first.hash == txid);`
`440`	`440`	`}`
`441`	`441`
`442`		`-void CWallet::Flush(bool shutdown)`
	`442`	`+void CWallet::Flush()`
`443`	`443`	`{`
`444`		`- database->Flush(shutdown);`
	`444`	`+ database->Flush();`
	`445`	`+}`
	`446`	`+`
	`447`	`+void CWallet::Close()`
	`448`	`+{`
	`449`	`+ database->Close();`
`445`	`450`	`}`
`446`	`451`
`447`	`452`	`void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> range)`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ namespace WalletTool {`
`17`	`17`	`static void WalletToolReleaseWallet(CWallet* wallet)`
`18`	`18`	`{`
`19`	`19`	`wallet->WalletLogPrintf("Releasing wallet\n");`
`20`		`- wallet->Flush(true);`
	`20`	`+ wallet->Close();`
`21`	`21`	`delete wallet;`
`22`	`22`	`}`
`23`	`23`
`@@ -133,7 +133,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)`
`133`	`133`	`std::shared_ptr<CWallet> wallet_instance = CreateWallet(name, path);`
`134`	`134`	`if (wallet_instance) {`
`135`	`135`	`WalletShowInfo(wallet_instance.get());`
`136`		`- wallet_instance->Flush(true);`
	`136`	`+ wallet_instance->Close();`
`137`	`137`	`}`
`138`	`138`	`} else if (command == "info" \|\| command == "salvage") {`
`139`	`139`	`if (!fs::exists(path)) {`
`@@ -145,7 +145,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)`
`145`	`145`	`std::shared_ptr<CWallet> wallet_instance = LoadWallet(name, path);`
`146`	`146`	`if (!wallet_instance) return false;`
`147`	`147`	`WalletShowInfo(wallet_instance.get());`
`148`		`- wallet_instance->Flush(true);`
	`148`	`+ wallet_instance->Close();`
`149`	`149`	`} else if (command == "salvage") {`
`150`	`150`	`return SalvageWallet(path);`
`151`	`151`	`}`