Merge #12422: util: Make LockDirectory thread-safe, consistent, and fix OpenBSD 6.2 build

1d4cbd2 test: Add unit test for LockDirectory (Wladimir J. van der Laan)
fc888bf util: Fix multiple use of LockDirectory (Wladimir J. van der Laan)

Pull request description:

  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 effect otherwise.

  Also add a unit test, make the function thread-safe, and fix Linux versus Windows behavior inconsistency.

  Meant to fix #12413.

Tree-SHA512: 1a94c714c932524a51212c46e8951c129337d57b00fd3da5a347c6bcf6a947706cd440f39df935591b2079995136917f71ca7435fb356f6e8a128c509a62ec32

1 files changed, 29 insertions, 6 deletions

diff --git a/src/util.cpp b/src/util.cpp
index 6738bbc6e..dcf7ed38b 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -373,20 +373,37 @@ int LogPrintStr(const std::string &str)
     return ret;
 }
 
+/** 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, or ReleaseDirectoryLocks
+ * is called.
+ */
+static std::map<std::string, std::unique_ptr<boost::interprocess::file_lock>> dir_locks;
+/** Mutex to protect dir_locks. */
+static std::mutex cs_dir_locks;
+
 bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only)
 {
+    std::lock_guard<std::mutex> ulock(cs_dir_locks);
     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 (dir_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
+            dir_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());
@@ -394,6 +411,12 @@ bool LockDirectory(const fs::path& directory, const std::string lockfile_name, b
     return true;
 }
 
+void ReleaseDirectoryLocks()
+{
+    std::lock_guard<std::mutex> ulock(cs_dir_locks);
+    dir_locks.clear();
+}
+
 /** Interpret string as boolean, for argument parsing */
 static bool InterpretBool(const std::string& strValue)
 {