diff --git a/src/init.cpp b/src/init.cpp index 9f86960ae51..96f51d17658 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1028,10 +1028,9 @@ static bool LockDataDirectory(bool probeOnly) { // Make sure only a single Bitcoin process is using the data directory. const fs::path& datadir = gArgs.GetDataDirNet(); - if (!DirIsWritable(datadir)) { - return InitError(strprintf(_("Cannot write to data directory '%s'; check permissions."), fs::PathToString(datadir))); - } switch (util::LockDirectory(datadir, ".lock", probeOnly)) { + case util::LockResult::ErrorWrite: + return InitError(strprintf(_("Cannot write to data directory '%s'; check permissions."), fs::PathToString(datadir))); 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)); case util::LockResult::Success: return true; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 3f79ad85535..4586e4814a5 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1112,6 +1112,7 @@ 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 + ResErrorWrite, ResErrorLock, ResUnlockSuccess, }; @@ -1127,6 +1128,7 @@ enum : char { ch = [&] { switch (util::LockDirectory(dirname, lockname)) { case util::LockResult::Success: return ResSuccess; + case util::LockResult::ErrorWrite: return ResErrorWrite; case util::LockResult::ErrorLock: return ResErrorLock; } // no default case, so the compiler can warn about missing cases assert(false); @@ -1171,9 +1173,15 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory) TestOtherProcess(dirname, lockname, fd[0]); } BOOST_CHECK_EQUAL(close(fd[0]), 0); // Parent: close child end + + char ch; + // Lock on non-existent directory should fail + BOOST_CHECK_EQUAL(write(fd[1], &LockCommand, 1), 1); + BOOST_CHECK_EQUAL(read(fd[1], &ch, 1), 1); + BOOST_CHECK_EQUAL(ch, ResErrorWrite); #endif // Lock on non-existent directory should fail - BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname), util::LockResult::ErrorLock); + BOOST_CHECK_EQUAL(util::LockDirectory(dirname, lockname), util::LockResult::ErrorWrite); fs::create_directories(dirname); @@ -1193,7 +1201,6 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory) 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(ch, ResErrorLock); diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp index a5408dd0d78..738d9bba449 100644 --- a/src/util/fs_helpers.cpp +++ b/src/util/fs_helpers.cpp @@ -65,8 +65,11 @@ LockResult LockDirectory(const fs::path& directory, const fs::path& lockfile_nam } // Create empty lock file if it doesn't exist. - FILE* file = fsbridge::fopen(pathLockFile, "a"); - if (file) fclose(file); + if (auto created{fsbridge::fopen(pathLockFile, "a")}) { + std::fclose(created); + } else { + return LockResult::ErrorWrite; + } auto lock = std::make_unique(pathLockFile); if (!lock->TryLock()) { error("Error while attempting to lock directory %s: %s", fs::PathToString(directory), lock->GetReason()); diff --git a/src/util/fs_helpers.h b/src/util/fs_helpers.h index 3e304820049..2d437b9c077 100644 --- a/src/util/fs_helpers.h +++ b/src/util/fs_helpers.h @@ -38,6 +38,7 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length); namespace util { enum class LockResult { Success, + ErrorWrite, ErrorLock, }; [[nodiscard]] LockResult LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only = false);