refactor locks

.cspell/words.txt

@@ -145,6 +145,7 @@ libuuid_include_dirs
 libvlc
 linkflags
 localappdata
+lpbyte
 lptr
 lpwstr
 markdownlint

repertory/librepertory/include/platform/unix_platform.hpp

@@ -26,11 +26,13 @@
 #include "types/repertory.hpp"
 
 namespace repertory {
+[[nodiscard]] auto create_lock_id(provider_type prov, std::string unique_id);
+
 class i_provider;
 
 class lock_data final {
 public:
-  explicit lock_data(const provider_type &prov, std::string unique_id /*= ""*/);
+  explicit lock_data(const provider_type &prov, std::string unique_id);
 
   lock_data(const lock_data &) = delete;
   lock_data(lock_data &&) = delete;
@@ -42,7 +44,6 @@ public:
 
 private:
   provider_type prov_;
-  std::string unique_id_;
   std::string mutex_id_;
   int lock_fd_;
   int lock_status_{EWOULDBLOCK};

repertory/librepertory/include/platform/win32_platform.hpp

@@ -23,20 +23,16 @@
 #define REPERTORY_INCLUDE_PLATFORM_WINPLATFORM_HPP_
 #if defined(_WIN32)
 
-#include "app_config.hpp"
 #include "types/repertory.hpp"
 
 namespace repertory {
+[[nodiscard]] auto create_lock_id(provider_type prov, std::string unique_id);
+
 class i_provider;
 
 class lock_data final {
 public:
-  explicit lock_data(const provider_type &prov, std::string unique_id /*= ""*/)
+  explicit lock_data(provider_type prov, std::string unique_id);
-      : prov_(prov),
-        unique_id_(std::move(unique_id)),
-        mutex_id_("repertory_" + app_config::get_provider_name(prov) + "_" +
-                  unique_id_),
-        mutex_handle_(::CreateMutex(nullptr, FALSE, mutex_id_.c_str())) {}
   lock_data(const lock_data &) = delete;
   lock_data(lock_data &&) = delete;
 
@@ -47,18 +43,14 @@ public:
 
 private:
   provider_type prov_;
-  std::string unique_id_;
   std::string mutex_id_;
-  HANDLE mutex_handle_{};
+  HANDLE mutex_handle_{INVALID_HANDLE_VALUE};
   DWORD mutex_state_{WAIT_FAILED};
 
+  [[nodiscard]] auto get_current_mount_state(json &mount_state) -> bool;
+
 public:
-  [[nodiscard]] auto get_mount_state(provider_type prov, json &mount_state)
+  [[nodiscard]] auto get_mount_state(json &mount_state) -> bool;
-      -> bool;
-
-  [[nodiscard]] static auto get_mount_state(json &mount_state) -> bool;
-
-  [[nodiscard]] auto get_unique_id() const -> std::string { return unique_id_; }
 
   [[nodiscard]] auto grab_lock(std::uint8_t retry_count = 30U) -> lock_result;
 

repertory/librepertory/src/platform/unix_platform.cpp

@@ -36,49 +36,58 @@
 #include "utils/unix.hpp"
 
 namespace repertory {
-lock_data::lock_data(const provider_type &prov, std::string unique_id /*= ""*/)
+auto create_lock_id(provider_type prov, std::string unique_id) {
-    : prov_(prov),
+  return fmt::format("{}_{}_{}", REPERTORY_DATA_NAME,
-      unique_id_(std::move(unique_id)),
+                     app_config::get_provider_name(prov), unique_id);
-      mutex_id_("repertory_" + app_config::get_provider_name(prov) + "_" +
+}
-                unique_id_) {
+
+lock_data::lock_data(const provider_type &prov, std::string unique_id)
+    : prov_(prov), mutex_id_(create_lock_id(prov, unique_id)) {
   lock_fd_ = open(get_lock_file().c_str(), O_CREAT | O_RDWR, S_IWUSR | S_IRUSR);
 }
 
 lock_data::~lock_data() { release(); }
 
 auto lock_data::get_lock_data_file() -> std::string {
-  const auto dir = get_state_directory();
+  auto dir = get_state_directory();
   if (not utils::file::directory(dir).create_directory()) {
     throw startup_exception("failed to create directory|sp|" + dir + "|err|" +
                             std::to_string(utils::get_last_error_code()));
   }
+
   return utils::path::combine(
-      dir, {"mountstate_" + std::to_string(getuid()) + ".json"});
+      dir, {
+               fmt::format("{}_{}.json", mutex_id_, getuid()),
+           });
 }
 
 auto lock_data::get_lock_file() -> std::string {
-  const auto dir = get_state_directory();
+  auto dir = get_state_directory();
   if (not utils::file::directory(dir).create_directory()) {
     throw startup_exception("failed to create directory|sp|" + dir + "|err|" +
                             std::to_string(utils::get_last_error_code()));
   }
 
-  return utils::path::combine(dir,
+  return utils::path::combine(
-                              {mutex_id_ + "_" + std::to_string(getuid())});
+      dir, {
+               fmt::format("{}_{}.lock", mutex_id_, getuid()),
+           });
 }
 
 auto lock_data::get_mount_state(json &mount_state) -> bool {
-  auto ret = false;
   auto fd =
       open(get_lock_data_file().c_str(), O_CREAT | O_RDWR, S_IWUSR | S_IRUSR);
-  if (fd != -1) {
+  if (fd == -1) {
-    if (wait_for_lock(fd) == 0) {
+    return false;
-      ret = utils::file::read_json_file(get_lock_data_file(), mount_state);
-      flock(fd, LOCK_UN);
-    }
-
-    close(fd);
   }
+
+  auto ret{false};
+  if (wait_for_lock(fd) == 0) {
+    ret = utils::file::read_json_file(get_lock_data_file(), mount_state);
+    flock(fd, LOCK_UN);
+  }
+
+  close(fd);
   return ret;
 }
 
@@ -93,8 +102,6 @@ auto lock_data::get_state_directory() -> std::string {
 }
 
 auto lock_data::grab_lock(std::uint8_t retry_count) -> lock_result {
-  REPERTORY_USES_FUNCTION_NAME();
-
   if (lock_fd_ == -1) {
     return lock_result::failure;
   }
@@ -116,7 +123,7 @@ void lock_data::release() {
   }
 
   if (lock_status_ == 0) {
-    unlink(get_lock_file().c_str());
+    utils::file::file{get_lock_file()}.delete();
     flock(lock_fd_, LOCK_UN);
   }
 
@@ -128,59 +135,55 @@ auto lock_data::set_mount_state(bool active, const std::string &mount_location,
                                 int pid) -> bool {
   REPERTORY_USES_FUNCTION_NAME();
 
-  auto ret = false;
   auto handle =
       open(get_lock_data_file().c_str(), O_CREAT | O_RDWR, S_IWUSR | S_IRUSR);
-  if (handle != -1) {
+  if (handle == -1) {
-    if (wait_for_lock(handle) == 0) {
+    return false;
-      const auto mount_id =
+  }
-          app_config::get_provider_display_name(prov_) + unique_id_;
-      json mount_state;
-      if (not utils::file::read_json_file(get_lock_data_file(), mount_state)) {
-        utils::error::raise_error(function_name,
-                                  "failed to read mount state file|sp|" +
-                                      get_lock_file());
-      }
-      if ((mount_state.find(mount_id) == mount_state.end()) ||
-          (mount_state[mount_id].find("Active") ==
-           mount_state[mount_id].end()) ||
-          (mount_state[mount_id]["Active"].get<bool>() != active) ||
-          (active && ((mount_state[mount_id].find("Location") ==
-                       mount_state[mount_id].end()) ||
-                      (mount_state[mount_id]["Location"].get<std::string>() !=
-                       mount_location)))) {
-        const auto lines = utils::file::read_file_lines(get_lock_data_file());
-        const auto txt = std::accumulate(
-            lines.begin(), lines.end(), std::string(),
-            [](auto &&val, auto &&line) -> auto { return val + line; });
-        auto json_data = json::parse(txt.empty() ? "{}" : txt);
-        json_data[mount_id] = {
-            {"Active", active},
-            {"Location", active ? mount_location : ""},
-            {"PID", active ? pid : -1},
-        };
-        if (mount_location.empty() && not active) {
-          ret = utils::file::file{get_lock_data_file()}.delete();
-        } else {
-          ret = utils::file::write_json_file(get_lock_data_file(), json_data);
-        }
-      } else {
-        ret = true;
-      }
 
-      flock(handle, LOCK_UN);
+  auto ret{false};
+  if (wait_for_lock(handle) == 0) {
+    json mount_state;
+    if (not utils::file::read_json_file(get_lock_data_file(), mount_state)) {
+      utils::error::raise_error(function_name,
+                                "failed to read mount state file|sp|" +
+                                    get_lock_file());
+    }
+    if ((mount_state.find("Active") == mount_state.end()) ||
+        (mount_state.get<bool>() != active) ||
+        (active && ((mount_state.find("Location") == mount_state.end()) ||
+                    (mount_state.get<std::string>() != mount_location)))) {
+      auto lines = utils::file::read_file_lines(get_lock_data_file());
+      auto txt = std::accumulate(
+          lines.begin(), lines.end(), std::string(),
+          [](auto &&val, auto &&line) -> auto { return val + line; });
+      auto json_data = json::parse(txt.empty() ? "{}" : txt);
+      json_data = {
+          {"Active", active},
+          {"Location", active ? mount_location : ""},
+          {"PID", active ? pid : -1},
+      };
+      if (mount_location.empty() && not active) {
+        ret = utils::file::file{get_lock_data_file()}.delete();
+      } else {
+        ret = utils::file::write_json_file(get_lock_data_file(), json_data);
+      }
+    } else {
+      ret = true;
     }
 
-    close(handle);
+    flock(handle, LOCK_UN);
   }
+
+  close(handle);
   return ret;
 }
 
 auto lock_data::wait_for_lock(int fd, std::uint8_t retry_count) -> int {
-  static constexpr const std::uint32_t max_sleep = 100U;
+  static constexpr const std::uint32_t max_sleep{100U};
 
-  auto lock_status = EWOULDBLOCK;
+  auto lock_status{EWOULDBLOCK};
-  auto remain = static_cast<std::uint32_t>(retry_count * max_sleep);
+  auto remain{static_cast<std::uint32_t>(retry_count * max_sleep)};
   while ((remain > 0) && (lock_status == EWOULDBLOCK)) {
     lock_status = flock(fd, LOCK_EX | LOCK_NB);
     if (lock_status == -1) {
@@ -231,7 +234,7 @@ auto provider_meta_handler(i_provider &provider, bool directory,
                            const api_file &file) -> api_error {
   REPERTORY_USES_FUNCTION_NAME();
 
-  const auto meta = create_meta_attributes(
+  auto meta = create_meta_attributes(
       file.accessed_date,
       directory ? FILE_ATTRIBUTE_DIRECTORY : FILE_ATTRIBUTE_ARCHIVE,
       file.changed_date, file.creation_date, directory, getgid(), file.key,

repertory/librepertory/src/platform/win32_platform.cpp

@@ -23,76 +23,79 @@
 
 #include "platform/win32_platform.hpp"
 
+#include "app_config.hpp"
 #include "events/event_system.hpp"
 #include "events/types/filesystem_item_added.hpp"
 #include "providers/i_provider.hpp"
 #include "utils/string.hpp"
 
 namespace repertory {
-auto lock_data::get_mount_state(provider_type /*pt*/, json &mount_state)
+auto create_lock_id(provider_type prov, std::string unique_id) {
-    -> bool {
+  return fmt::format("{}_{}_{}", REPERTORY_DATA_NAME,
-  if (not get_mount_state(mount_state)) {
+                     app_config::get_provider_name(prov), unique_id);
-    return false;
-  }
-
-  auto mount_id = app_config::get_provider_display_name(prov_) + unique_id_;
-  mount_state = mount_state[mount_id].empty() ? json({
-                                                    {"Active", false},
-                                                    {"Location", ""},
-                                                    {"PID", -1},
-                                                })
-                                              : mount_state[mount_id];
-
-  return true;
 }
 
-auto lock_data::get_mount_state(json &mount_state) -> bool {
+lock_data::lock_data(provider_type prov, std::string unique_id)
+    : prov_(prov),
+      mutex_id_(create_lock_id(prov, unique_id)),
+      mutex_handle_(::CreateMutex(nullptr, FALSE,
+                                  create_lock_id(prov, unique_id).c_str())) {}
+
+auto lock_data::get_current_mount_state(json &mount_state) -> bool {
   HKEY key{};
-  auto ret = (::RegCreateKeyEx(
+  auto ret = (::RegCreateKeyExA(HKEY_CURRENT_USER,
-                  HKEY_CURRENT_USER,
+                                fmt::format(R"(SOFTWARE\{}\Mounts\\{})",
-                  ("SOFTWARE\\" + std::string{REPERTORY_DATA_NAME} + "\\Mounts")
+                                            REPERTORY_DATA_NAME, mutex_id_)
-                      .c_str(),
+                                    .c_str(),
-                  0, nullptr, 0, KEY_ALL_ACCESS, nullptr, &key,
+                                0, nullptr, 0, KEY_ALL_ACCESS, nullptr, &key,
-                  nullptr) == ERROR_SUCCESS);
+                                nullptr) == ERROR_SUCCESS);
   if (not ret) {
     return ret;
   }
 
   std::string data;
   DWORD data_size{};
-  DWORD idx{};
-  std::string name;
-  name.resize(32767U);
-  auto name_size{static_cast<DWORD>(name.size())};
-  while (ret &&
-         (::RegEnumValue(key, idx, name.data(), &name_size, nullptr, nullptr,
-                         nullptr, &data_size) == ERROR_SUCCESS)) {
 
-    data.resize(data_size);
+  DWORD type{REG_SZ};
-    ret = (::RegEnumValue(key, idx, name.data(), &name_size, nullptr, nullptr,
+  ::RegGetValueA(key, nullptr, nullptr, 0, &type, data.data(), &data_size);
-                          reinterpret_cast<LPBYTE>(data.data()),
-                          &data_size) == ERROR_SUCCESS);
-    if (ret) {
-      mount_state[name] = json::parse(data);
-      name_size = static_cast<DWORD>(name.size());
-      data_size = 0U;
-    }
 
-    ++idx;
+  data.resize(data_size);
+  ret = (::RegGetValueA(key, nullptr, nullptr, 0, &type, data.data(),
+                        &data_size) == ERROR_SUCCESS);
+  if (ret && data_size != 0U) {
+    mount_state = json::parse(data);
   }
 
   ::RegCloseKey(key);
   return ret;
 }
 
+auto lock_data::get_mount_state(json &mount_state) -> bool {
+  if (not get_current_mount_state(mount_state)) {
+    return false;
+  }
+
+  mount_state = mount_state.empty() ? json({
+                                          {"Active", false},
+                                          {"Location", ""},
+                                          {"PID", -1},
+                                      })
+                                    : mount_state;
+
+  return true;
+}
+
 auto lock_data::grab_lock(std::uint8_t retry_count) -> lock_result {
+  static constexpr const std::uint32_t max_sleep{100U};
+
   if (mutex_handle_ == INVALID_HANDLE_VALUE) {
     return lock_result::failure;
   }
 
   for (std::uint8_t idx = 0U;
-       (idx <= retry_count) && ((mutex_state_ = ::WaitForSingleObject(
+       (idx <= retry_count) &&
-                                     mutex_handle_, 100)) == WAIT_TIMEOUT);
+       ((mutex_state_ = ::WaitForSingleObject(mutex_handle_, max_sleep)) ==
+        WAIT_TIMEOUT);
        ++idx) {
   }
 
@@ -127,28 +130,22 @@ auto lock_data::set_mount_state(bool active, std::string_view mount_location,
     return false;
   }
 
-  auto mount_id{app_config::get_provider_display_name(prov_) + unique_id_};
-
   json mount_state;
   [[maybe_unused]] auto success{get_mount_state(mount_state)};
-  if (not((mount_state.find(mount_id) == mount_state.end()) ||
+  if (not((mount_state.find("Active") == mount_state.end()) ||
-          (mount_state[mount_id].find("Active") ==
+          (mount_state.get<bool>() != active) ||
-           mount_state[mount_id].end()) ||
+          (active && ((mount_state.find("Location") == mount_state.end()) ||
-          (mount_state[mount_id]["Active"].get<bool>() != active) ||
+                      (mount_state.get<std::string>() != mount_location))))) {
-          (active && ((mount_state[mount_id].find("Location") ==
-                       mount_state[mount_id].end()) ||
-                      (mount_state[mount_id]["Location"].get<std::string>() !=
-                       mount_location))))) {
     return true;
   }
 
   HKEY key{};
-  if (::RegCreateKeyEx(
+  if (::RegCreateKeyExA(HKEY_CURRENT_USER,
-          HKEY_CURRENT_USER,
+                        fmt::format(R"(SOFTWARE\{}\Mounts\\{})",
-          ("SOFTWARE\\" + std::string{REPERTORY_DATA_NAME} + "\\Mounts")
+                                    REPERTORY_DATA_NAME, mutex_id_)
-              .c_str(),
+                            .c_str(),
-          0, nullptr, 0, KEY_ALL_ACCESS, nullptr, &key,
+                        0, nullptr, 0, KEY_ALL_ACCESS, nullptr, &key,
-          nullptr) != ERROR_SUCCESS) {
+                        nullptr) != ERROR_SUCCESS) {
     return false;
   }
 
@@ -163,14 +160,24 @@ auto lock_data::set_mount_state(bool active, std::string_view mount_location,
 
   auto ret{false};
   if (mount_location.empty() && not active) {
-    ret = (::RegDeleteKey(key, mount_id.c_str()) == ERROR_SUCCESS);
+    ::RegCloseKey(key);
+
+    if (::RegCreateKeyExA(
+            HKEY_CURRENT_USER,
+            fmt::format(R"(SOFTWARE\{}\Mounts)", REPERTORY_DATA_NAME).c_str(),
+            0, nullptr, 0, KEY_ALL_ACCESS, nullptr, &key,
+            nullptr) != ERROR_SUCCESS) {
+      return false;
+    }
+
+    ret = (::RegDeleteKeyA(key, mutex_id_.c_str()) == ERROR_SUCCESS);
   } else {
-    ret = (::RegSetValueEx(key, mount_id.c_str(), 0, REG_SZ,
+    ret = (::RegSetValueEx(key, nullptr, 0, REG_SZ,
                            reinterpret_cast<const BYTE *>(data.c_str()),
                            static_cast<DWORD>(data.size())) == ERROR_SUCCESS);
   }
-  ::RegCloseKey(key);
 
+  ::RegCloseKey(key);
   return ret;
 }
 
@@ -221,4 +228,4 @@ auto provider_meta_handler(i_provider &provider, bool directory,
 }
 } // namespace repertory
 
-#endif //_WIN32
+#endif // defined(_WIN32)

repertory/repertory_test/src/lock_data_test.cpp

@@ -62,13 +62,16 @@ TEST(lock_data_test, set_and_unset_mount_state) {
 
   json mount_state;
   EXPECT_TRUE(l.get_mount_state(mount_state));
-
   EXPECT_STREQ(R"({"Active":true,"Location":"C:","PID":99})",
-               mount_state["Sia1"].dump().c_str());
+               mount_state.dump().c_str());
+
+  EXPECT_TRUE(l2.get_mount_state(mount_state));
   EXPECT_STREQ(R"({"Active":true,"Location":"D:","PID":97})",
-               mount_state["Remote1"].dump().c_str());
+               mount_state.dump().c_str());
+
+  EXPECT_TRUE(l3.get_mount_state(mount_state));
   EXPECT_STREQ(R"({"Active":true,"Location":"E:","PID":96})",
-               mount_state["Remote2"].dump().c_str());
+               mount_state.dump().c_str());
 
   EXPECT_TRUE(l.set_mount_state(false, "C:", 99));
   EXPECT_TRUE(l2.set_mount_state(false, "D:", 98));
@@ -76,11 +79,15 @@ TEST(lock_data_test, set_and_unset_mount_state) {
 
   EXPECT_TRUE(l.get_mount_state(mount_state));
   EXPECT_STREQ(R"({"Active":false,"Location":"","PID":-1})",
-               mount_state["Sia1"].dump().c_str());
+               mount_state.dump().c_str());
+
+  EXPECT_TRUE(l2.get_mount_state(mount_state));
   EXPECT_STREQ(R"({"Active":false,"Location":"","PID":-1})",
-               mount_state["Remote1"].dump().c_str());
+               mount_state.dump().c_str());
+
+  EXPECT_TRUE(l3.get_mount_state(mount_state));
   EXPECT_STREQ(R"({"Active":false,"Location":"","PID":-1})",
-               mount_state["Remote2"].dump().c_str());
+               mount_state.dump().c_str());
 }
 #else
 TEST(lock_data_test, set_and_unset_mount_state) {
@@ -91,14 +98,13 @@ TEST(lock_data_test, set_and_unset_mount_state) {
   EXPECT_TRUE(l.get_mount_state(mount_state));
 
   EXPECT_STREQ(R"({"Active":true,"Location":"/mnt/1","PID":99})",
-               mount_state["Sia1"].dump().c_str());
+               mount_state.dump().c_str());
 
   EXPECT_TRUE(l.set_mount_state(false, "/mnt/1", 99));
 
   EXPECT_TRUE(l.get_mount_state(mount_state));
-
   EXPECT_STREQ(R"({"Active":false,"Location":"","PID":-1})",
-               mount_state["Sia1"].dump().c_str());
+               mount_state.dump().c_str());
 }
 #endif
 } // namespace repertory