Merge #11476: Avoid opening copied wallet databases simultaneously
478a89c
Avoid opening copied wallet databases simultaneously (Russell Yanofsky)
Pull request description:
Make sure wallet databases have unique fileids. If they don't, throw an error. BDB caches do not work properly when more than one open database has the same fileid, because values written to one database may show up in reads to other databases.
Bitcoin will never create different databases with the same fileid, but users can create them by manually copying database files.
BDB caching bug was reported by @dooglus in https://github.com/bitcoin/bitcoin/issues/11429
Tree-SHA512: e7635dc81a181801f42324b72fe9e0a2a7dd00b1dcf5abcbf27fa50938eb9a1fc3065c2321326c3456c48c29ae6504353b02f3d46e6eb2f7b09e46d8fe24388d
This commit is contained in:
commit
99e93de6f8
2 changed files with 41 additions and 0 deletions
|
@ -20,6 +20,40 @@
|
||||||
|
|
||||||
#include <boost/thread.hpp>
|
#include <boost/thread.hpp>
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
//! Make sure database has a unique fileid within the environment. If it
|
||||||
|
//! doesn't, throw an error. BDB caches do not work properly when more than one
|
||||||
|
//! open database has the same fileid (values written to one database may show
|
||||||
|
//! up in reads to other databases).
|
||||||
|
//!
|
||||||
|
//! BerkeleyDB generates unique fileids by default
|
||||||
|
//! (https://docs.oracle.com/cd/E17275_01/html/programmer_reference/program_copy.html),
|
||||||
|
//! so bitcoin should never create different databases with the same fileid, but
|
||||||
|
//! this error can be triggered if users manually copy database files.
|
||||||
|
void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db)
|
||||||
|
{
|
||||||
|
if (env.IsMock()) return;
|
||||||
|
|
||||||
|
u_int8_t fileid[DB_FILE_ID_LEN];
|
||||||
|
int ret = db.get_mpf()->get_fileid(fileid);
|
||||||
|
if (ret != 0) {
|
||||||
|
throw std::runtime_error(strprintf("CDB: Can't open database %s (get_fileid failed with %d)", filename, ret));
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const auto& item : env.mapDb) {
|
||||||
|
u_int8_t item_fileid[DB_FILE_ID_LEN];
|
||||||
|
if (item.second && item.second->get_mpf()->get_fileid(item_fileid) == 0 &&
|
||||||
|
memcmp(fileid, item_fileid, sizeof(fileid)) == 0) {
|
||||||
|
const char* item_filename = nullptr;
|
||||||
|
item.second->get_dbname(&item_filename, nullptr);
|
||||||
|
throw std::runtime_error(strprintf("CDB: Can't open database %s (duplicates fileid %s from %s)", filename,
|
||||||
|
HexStr(std::begin(item_fileid), std::end(item_fileid)),
|
||||||
|
item_filename ? item_filename : "(unknown database)"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} // namespace
|
||||||
|
|
||||||
//
|
//
|
||||||
// CDB
|
// CDB
|
||||||
//
|
//
|
||||||
|
@ -403,6 +437,7 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb
|
||||||
if (ret != 0) {
|
if (ret != 0) {
|
||||||
throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename));
|
throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename));
|
||||||
}
|
}
|
||||||
|
CheckUniqueFileid(*env, strFilename, *pdb_temp);
|
||||||
|
|
||||||
pdb = pdb_temp.release();
|
pdb = pdb_temp.release();
|
||||||
env->mapDb[strFilename] = pdb;
|
env->mapDb[strFilename] = pdb;
|
||||||
|
|
|
@ -7,6 +7,7 @@
|
||||||
Verify that a bitcoind node can load multiple wallet files
|
Verify that a bitcoind node can load multiple wallet files
|
||||||
"""
|
"""
|
||||||
import os
|
import os
|
||||||
|
import shutil
|
||||||
|
|
||||||
from test_framework.test_framework import BitcoinTestFramework
|
from test_framework.test_framework import BitcoinTestFramework
|
||||||
from test_framework.util import assert_equal, assert_raises_rpc_error
|
from test_framework.util import assert_equal, assert_raises_rpc_error
|
||||||
|
@ -29,6 +30,11 @@ class MultiWalletTest(BitcoinTestFramework):
|
||||||
os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w11'))
|
os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w11'))
|
||||||
self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.')
|
self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.')
|
||||||
|
|
||||||
|
# should not initialize if one wallet is a copy of another
|
||||||
|
shutil.copyfile(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w2'),
|
||||||
|
os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w22'))
|
||||||
|
self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid')
|
||||||
|
|
||||||
# should not initialize if wallet file is a symlink
|
# should not initialize if wallet file is a symlink
|
||||||
os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'), os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12'))
|
os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'), os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12'))
|
||||||
self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.')
|
self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.')
|
||||||
|
|
Loading…
Reference in a new issue