foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Code Issues Activity

refactor: Return enum in LockDirectory

Browse source 
This makes it easier to add more Error cases in the future. Also, add
missing util namespace.
This commit is contained in:
MarcoFalke 2023-07-14 11:26:45 +02:00
parent 64879f4c03
commit fa0afe7408
No known key found for this signature in database
5 changed files with 48 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
src/init.cpp
View file

@ -1031,10 +1031,12 @@ static bool LockDataDirectory(bool probeOnly)
if (!DirIsWritable(datadir)) {
return InitError(strprintf(_("Cannot write to data directory '%s'; check permissions."), fs::PathToString(datadir)));
}
if (!LockDirectory(datadir, ".lock", probeOnly)) {
switch (util::LockDirectory(datadir, ".lock", probeOnly)) {
case util::LockResult::ErrorLock:
return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), fs::PathToString(datadir), PACKAGE_NAME));
}
return true;
case util::LockResult::Success: return true;
} // no default case, so the compiler can warn about missing cases
assert(false);
}
bool AppInitSanityChecks(const kernel::Context& kernel)

50
src/test/util_tests.cpp
View file

@ -1106,15 +1106,15 @@ BOOST_AUTO_TEST_CASE(test_ParseFixedPoint)
BOOST_CHECK(!ParseFixedPoint("31.999999999999999999999", 3, &amount));
}
static void TestOtherThread(fs::path dirname, fs::path lockname, bool *result)
{
*result = LockDirectory(dirname, lockname);
}
#ifndef WIN32 // Cannot do this test on WIN32 due to lack of fork()
static constexpr char LockCommand = 'L';
static constexpr char UnlockCommand = 'U';
static constexpr char ExitCommand = 'X';
enum : char {
ResSuccess = 2, // Start with 2 to avoid accidental collision with common values 0 and 1
ResErrorLock,
ResUnlockSuccess,
};
[[noreturn]] static void TestOtherProcess(fs::path dirname, fs::path lockname, int fd)
{
@ -1122,15 +1122,21 @@ static constexpr char ExitCommand = 'X';
while (true) {
int rv = read(fd, &ch, 1); // Wait for command
assert(rv == 1);
switch(ch) {
switch (ch) {
case LockCommand:
ch = LockDirectory(dirname, lockname);
ch = [&] {
switch (util::LockDirectory(dirname, lockname)) {
case util::LockResult::Success: return ResSuccess;
case util::LockResult::ErrorLock: return ResErrorLock;
} // no default case, so the compiler can warn about missing cases
assert(false);
}();
rv = write(fd, &ch, 1);
assert(rv == 1);
break;
case UnlockCommand:
ReleaseDirectoryLocks();
ch = true; // Always succeeds
ch = ResUnlockSuccess; // Always succeeds
rv = write(fd, &ch, 1);
assert(rv == 1);
break;
@ -1167,51 +1173,51 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory)
BOOST_CHECK_EQUAL(close(fd[0]), 0); // Parent: close child end
#endif
// Lock on non-existent directory should fail
BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname), false);
BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname), util::LockResult::ErrorLock);
fs::create_directories(dirname);
// Probing lock on new directory should succeed
BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), true);
BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname, true), util::LockResult::Success);
// Persistent lock on new directory should succeed
BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname), true);
BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname), util::LockResult::Success);
// Another lock on the directory from the same thread should succeed
BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname), true);
BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname), util::LockResult::Success);
// Another lock on the directory from a different thread within the same process should succeed
bool threadresult;
std::thread thr(TestOtherThread, dirname, lockname, &threadresult);
util::LockResult threadresult;
std::thread thr([&] { threadresult = util::LockDirectory(dirname, lockname); });
thr.join();
BOOST_CHECK_EQUAL(threadresult, true);
BOOST_CHECK_EQUAL(threadresult, util::LockResult::Success);
#ifndef WIN32
// Try to acquire lock in child process while we're holding it, this should fail.
char ch;
BOOST_CHECK_EQUAL(write(fd[1], &LockCommand, 1), 1);
BOOST_CHECK_EQUAL(read(fd[1], &ch, 1), 1);
BOOST_CHECK_EQUAL((bool)ch, false);
BOOST_CHECK_EQUAL(ch, ResErrorLock);
// Give up our lock
ReleaseDirectoryLocks();
// Probing lock from our side now should succeed, but not hold on to the lock.
BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), true);
BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname, true), util::LockResult::Success);
// Try to acquire the lock in the child process, this should be successful.
BOOST_CHECK_EQUAL(write(fd[1], &LockCommand, 1), 1);
BOOST_CHECK_EQUAL(read(fd[1], &ch, 1), 1);
BOOST_CHECK_EQUAL((bool)ch, true);
BOOST_CHECK_EQUAL(ch, ResSuccess);
// When we try to probe the lock now, it should fail.
BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), false);
BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname, true), util::LockResult::ErrorLock);
// Unlock the lock in the child process
BOOST_CHECK_EQUAL(write(fd[1], &UnlockCommand, 1), 1);
BOOST_CHECK_EQUAL(read(fd[1], &ch, 1), 1);
BOOST_CHECK_EQUAL((bool)ch, true);
BOOST_CHECK_EQUAL(ch, ResUnlockSuccess);
// When we try to probe the lock now, it should succeed.
BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), true);
BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname, true), util::LockResult::Success);
// Re-lock the lock in the child process, then wait for it to exit, check
// successful return. After that, we check that exiting the process
@ -1221,7 +1227,7 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory)
BOOST_CHECK_EQUAL(write(fd[1], &ExitCommand, 1), 1);
BOOST_CHECK_EQUAL(waitpid(pid, &processstatus, 0), pid);
BOOST_CHECK_EQUAL(processstatus, 0);
BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), true);
BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname, true), util::LockResult::Success);
// Restore SIGCHLD
signal(SIGCHLD, old_handler);

13
src/util/fs_helpers.cpp
View file

@ -53,15 +53,15 @@ static GlobalMutex cs_dir_locks;
* is called.
*/
static std::map<std::string, std::unique_ptr<fsbridge::FileLock>> dir_locks GUARDED_BY(cs_dir_locks);
bool LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only)
namespace util {
LockResult LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only)
{
LOCK(cs_dir_locks);
fs::path pathLockFile = directory / lockfile_name;
// If a lock for this directory already exists in the map, don't try to re-lock it
if (dir_locks.count(fs::PathToString(pathLockFile))) {
return true;
return LockResult::Success;
}
// Create empty lock file if it doesn't exist.
@ -69,15 +69,16 @@ bool LockDirectory(const fs::path& directory, const fs::path& lockfile_name, boo
if (file) fclose(file);
auto lock = std::make_unique<fsbridge::FileLock>(pathLockFile);
if (!lock->TryLock()) {
return error("Error while attempting to lock directory %s: %s", fs::PathToString(directory), lock->GetReason());
error("Error while attempting to lock directory %s: %s", fs::PathToString(directory), lock->GetReason());
return LockResult::ErrorLock;
}
if (!probe_only) {
// Lock successful and we're not just probing, put it into the map
dir_locks.emplace(fs::PathToString(pathLockFile), std::move(lock));
}
return true;
return LockResult::Success;
}
} // namespace util
void UnlockDirectory(const fs::path& directory, const fs::path& lockfile_name)
{
LOCK(cs_dir_locks);

8
src/util/fs_helpers.h
View file

@ -35,7 +35,13 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length);
*/
[[nodiscard]] bool RenameOver(fs::path src, fs::path dest);
bool LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only = false);
namespace util {
enum class LockResult {
Success,
ErrorLock,
};
[[nodiscard]] LockResult LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only = false);
} // namespace util
void UnlockDirectory(const fs::path& directory, const fs::path& lockfile_name);
bool DirIsWritable(const fs::path& directory);
bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes = 0);

2
src/wallet/bdb.cpp
View file

@ -149,7 +149,7 @@ bool BerkeleyEnvironment::Open(bilingual_str& err)
fs::path pathIn = fs::PathFromString(strPath);
TryCreateDirectories(pathIn);
if (!LockDirectory(pathIn, ".walletlock")) {
if (util::LockDirectory(pathIn, ".walletlock") != util::LockResult::Success) {
LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance may be using it.\n", strPath);
err = strprintf(_("Error initializing wallet database environment %s!"), fs::quoted(fs::PathToString(Directory())));
return false;