No need to check for duplicate fileids in all dbenvs

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.
pull/764/head
Andrew Chow 4 years ago
parent d86efab370
commit 00f0041351

@ -32,12 +32,12 @@ void CheckUniqueFileid(const BerkeleyEnvironment& env, const std::string& filena
int ret = db.get_mpf()->get_fileid(fileid.value); int ret = db.get_mpf()->get_fileid(fileid.value);
if (ret != 0) { 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) { for (const auto& item : env.m_fileids) {
if (fileid == item.second && &fileid != &item.second) { if (fileid == item.second && &fileid != &item.second) {
throw std::runtime_error(strprintf("BerkeleyBatch: Can't open database %s (duplicates fileid %s from %s)", filename, throw std::runtime_error(strprintf("BerkeleyDatabase: Can't open database %s (duplicates fileid %s from %s)", filename,
HexStr(std::begin(item.second.value), std::end(item.second.value)), item.first)); HexStr(std::begin(item.second.value), std::end(item.second.value)), item.first));
} }
} }
@ -309,6 +309,8 @@ BerkeleyDatabase::~BerkeleyDatabase()
{ {
if (env) { if (env) {
LOCK(cs_db); LOCK(cs_db);
env->CloseDb(strFile);
assert(!m_db);
size_t erased = env->m_databases.erase(strFile); size_t erased = env->m_databases.erase(strFile);
assert(erased == 1); assert(erased == 1);
env->m_fileids.erase(strFile); env->m_fileids.erase(strFile);
@ -373,25 +375,12 @@ void BerkeleyDatabase::Open(const char* pszMode)
if (ret != 0) { if (ret != 0) {
throw std::runtime_error(strprintf("BerkeleyDatabase: Error %d, can't open database %s", ret, strFile)); throw std::runtime_error(strprintf("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 // Call CheckUniqueFileid on the containing BDB environment to
// avoid BDB data consistency bugs that happen when different data // avoid BDB data consistency bugs that happen when different data
// files in the same environment have the same fileid. // files in the same environment have the same fileid.
// CheckUniqueFileid(*env, strFile, *pdb_temp, this->env->m_fileids[strFile]);
// 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& env : g_dbenvs) {
CheckUniqueFileid(*env.second.lock().get(), strFile, *pdb_temp, this->env->m_fileids[strFile]);
}
m_db.reset(pdb_temp.release()); m_db.reset(pdb_temp.release());

@ -120,7 +120,7 @@ class MultiWalletTest(BitcoinTestFramework):
# should not initialize if one wallet is a copy of another # should not initialize if one wallet is a copy of another
shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) 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) self.nodes[0].assert_start_raises_init_error(['-wallet=w8', '-wallet=w8_copy'], exp_stderr, match=ErrorMatch.PARTIAL_REGEX)
# should not initialize if wallet file is a symlink # should not initialize if wallet file is a symlink
@ -258,10 +258,10 @@ class MultiWalletTest(BitcoinTestFramework):
assert_raises_rpc_error(-4, "Wallet file verification failed. Error loading wallet wallet.dat. Duplicate -wallet filename specified.", self.nodes[0].loadwallet, 'wallet.dat') assert_raises_rpc_error(-4, "Wallet file verification failed. Error loading wallet wallet.dat. Duplicate -wallet filename specified.", self.nodes[0].loadwallet, 'wallet.dat')
# Fail to load if one wallet is a copy of another # 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", self.nodes[0].loadwallet, 'w8_copy') 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 #14304 # Fail to load if one wallet is a copy of another, test this twice to make sure that we don't re-introduce #14304
assert_raises_rpc_error(-4, "BerkeleyBatch: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy') 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 # Fail to load if wallet file is a symlink

Loading…
Cancel
Save