Skip to content

Commit

Permalink
util: Fix multiple use of LockDirectory
Browse files Browse the repository at this point in the history
This commit fixes problems with calling LockDirectory multiple times on
the same directory, or from multiple threads. It also fixes the build on
OpenBSD.

- Wrap the boost::interprocess::file_lock in a std::unique_ptr inside
  the map that keeps track of per-directory locks. This fixes a build
  issue with the clang 4.0.0+boost-1.58.0p8 version combo on OpenBSD
  6.2, and should have no observable effect otherwise.

- Protect the locks map using a mutex.

- Make sure that only locks that are successfully acquired are inserted
  in the map.

- Open the lock file for appending only if we know we don't have the
  lock yet - The `FILE* file = fsbridge::fopen(pathLockFile, "a");`
  wipes the 'we own this lock' administration, likely because it opens
  a new fd for the locked file then closes it.
  • Loading branch information
laanwj committed Feb 15, 2018
1 parent f4f4f51 commit fc888bf
Showing 1 changed file with 20 additions and 6 deletions.
26 changes: 20 additions & 6 deletions src/util.cpp
Expand Up @@ -375,18 +375,32 @@ int LogPrintStr(const std::string &str)

bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only)
{
// A map that contains all the currently held directory locks. After
// successful locking, these will be held here until the global
// destructor cleans them up and thus automatically unlocks them.
static std::map<std::string, std::unique_ptr<boost::interprocess::file_lock>> locks;
// Protect the map with a mutex
static std::mutex cs;
std::lock_guard<std::mutex> ulock(cs);
fs::path pathLockFile = directory / lockfile_name;
FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist.

// If a lock for this directory already exists in the map, don't try to re-lock it
if (locks.count(pathLockFile.string())) {
return true;
}

// Create empty lock file if it doesn't exist.
FILE* file = fsbridge::fopen(pathLockFile, "a");
if (file) fclose(file);

try {
static std::map<std::string, boost::interprocess::file_lock> locks;
boost::interprocess::file_lock& lock = locks.emplace(pathLockFile.string(), pathLockFile.string().c_str()).first->second;
if (!lock.try_lock()) {
auto lock = MakeUnique<boost::interprocess::file_lock>(pathLockFile.string().c_str());
if (!lock->try_lock()) {
return false;
}
if (probe_only) {
lock.unlock();
if (!probe_only) {
// Lock successful and we're not just probing, put it into the map
locks.emplace(pathLockFile.string(), std::move(lock));
}
} catch (const boost::interprocess::interprocess_exception& e) {
return error("Error while attempting to lock directory %s: %s", directory.string(), e.what());
Expand Down

0 comments on commit fc888bf

Please sign in to comment.