No need to check for duplicate fileids in all dbenvs

Summary:
Since we have .walletlock in each directory, we don't need the duplicate
fileid checks across all dbenvs as it shouldn't be possible anyways.

This is a backport of [[bitcoin/bitcoin#19335 | core#19335]] [4/5]
bitcoin/bitcoin@00f0041

Depends on D10036

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D10037

src/wallet/bdb.cpp

@@ -33,17 +33,19 @@ void CheckUniqueFileid(const BerkeleyEnvironment &env,
 
     int ret = db.get_mpf()->get_fileid(fileid.value);
     if (ret != 0) {
-        throw std::runtime_error(strprintf(
-            "BerkeleyBatch: Can't open database %s (get_fileid failed with %d)",
-            filename, ret));
+        throw std::runtime_error(
+            strprintf("BerkeleyDatabase: Can't open database %s (get_fileid "
+                      "failed with %d)",
+                      filename, ret));
     }
 
     for (const auto &item : env.m_fileids) {
         if (fileid == item.second && &fileid != &item.second) {
-            throw std::runtime_error(strprintf(
-                "BerkeleyBatch: Can't open database %s (duplicates fileid %s "
-                "from %s)",
-                filename, HexStr(item.second.value), item.first));
+            throw std::runtime_error(
+                strprintf("BerkeleyDatabase: Can't open database %s "
+                          "(duplicates fileid %s "
+                          "from %s)",
+                          filename, HexStr(item.second.value), item.first));
         }
     }
 }
@@ -331,6 +333,8 @@ void BerkeleyEnvironment::CheckpointLSN(const std::string &strFile) {
 BerkeleyDatabase::~BerkeleyDatabase() {
     if (env) {
         LOCK(cs_db);
+        env->CloseDb(strFile);
+        assert(!m_db);
         size_t erased = env->m_databases.erase(strFile);
         assert(erased == 1);
         env->m_fileids.erase(strFile);
@@ -406,26 +410,13 @@ void BerkeleyDatabase::Open(const char *pszMode) {
                     "BerkeleyDatabase: Error %d, can't open database %s", ret,
                     strFile));
             }
+            m_file_path = (env->Directory() / strFile).string();
 
             // Call CheckUniqueFileid on the containing BDB environment to
             // avoid BDB data consistency bugs that happen when different data
             // files in the same environment have the same fileid.
-            //
-            // Also call CheckUniqueFileid on all the other g_dbenvs to prevent
-            // bitcoin from opening the same data file through another
-            // environment when the file is referenced through equivalent but
-            // not obviously identical symlinked or hard linked or bind mounted
-            // paths. In the future a more relaxed check for equal inode and
-            // device ids could be done instead, which would allow opening
-            // different backup copies of a wallet at the same time. Maybe even
-            // more ideally, an exclusive lock for accessing the database could
-            // be implemented, so no equality checks are needed at all. (Newer
-            // versions of BDB have an set_lk_exclusive method for this
-            // purpose, but the older version we use does not.)
-            for (const auto &dbenv : g_dbenvs) {
-                CheckUniqueFileid(*dbenv.second.lock().get(), strFile,
-                                  *pdb_temp, this->env->m_fileids[strFile]);
-            }
+            CheckUniqueFileid(*env, strFile, *pdb_temp,
+                              this->env->m_fileids[strFile]);
 
             m_db.reset(pdb_temp.release());
         }

test/functional/wallet_multiwallet.py

@@ -138,7 +138,7 @@ def wallet_file(name):
 
         # should not initialize if one wallet is a copy of another
         shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy'))
-        exp_stderr = r"BerkeleyBatch: Can't open database w8_copy \(duplicates fileid \w+ from w8\)"
+        exp_stderr = r"BerkeleyDatabase: Can't open database w8_copy \(duplicates fileid \w+ from w8\)"
         self.nodes[0].assert_start_raises_init_error(
             ['-wallet=w8', '-wallet=w8_copy'], exp_stderr, match=ErrorMatch.PARTIAL_REGEX)
 
@@ -317,13 +317,13 @@ def wallet_file(name):
             'wallet.dat')
 
         # Fail to load if one wallet is a copy of another
-        assert_raises_rpc_error(-4, "BerkeleyBatch: Can't open database w8_copy (duplicates fileid",
+        assert_raises_rpc_error(-4, "BerkeleyDatabase: Can't open database w8_copy (duplicates fileid",
                                 self.nodes[0].loadwallet, 'w8_copy')
 
         # Fail to load if one wallet is a copy of another.
         # Test this twice to make sure that we don't re-introduce
         # https://github.com/bitcoin/bitcoin/issues/14304
-        assert_raises_rpc_error(-4, "BerkeleyBatch: Can't open database w8_copy (duplicates fileid",
+        assert_raises_rpc_error(-4, "BerkeleyDatabase: Can't open database w8_copy (duplicates fileid",
                                 self.nodes[0].loadwallet, 'w8_copy')
 
         # Fail to load if wallet file is a symlink