troubleshoot deadlock

repertory/librepertory/include/file_manager/direct_open_file.hpp

@@ -42,10 +42,11 @@ public:
   direct_open_file(const direct_open_file &) noexcept = delete;
   direct_open_file(direct_open_file &&) noexcept = delete;
   auto operator=(direct_open_file &&) noexcept -> direct_open_file & = delete;
-  auto
+  auto operator=(const direct_open_file &) noexcept
-  operator=(const direct_open_file &) noexcept -> direct_open_file & = delete;
+      -> direct_open_file & = delete;
 
 private:
+  std::atomic<std::uint64_t> last_progress_{0U};
   std::size_t total_chunks_;
   stop_type stop_requested_{false};
 
@@ -63,13 +64,13 @@ public:
     return false;
   }
 
-  [[nodiscard]] auto
+  [[nodiscard]] auto get_read_state() const
-  get_read_state() const -> boost::dynamic_bitset<> override {
+      -> boost::dynamic_bitset<> override {
     return {};
   }
 
-  [[nodiscard]] auto
+  [[nodiscard]] auto get_read_state(std::size_t /* chunk */) const
-  get_read_state(std::size_t /* chunk */) const -> bool override {
+      -> bool override {
     return false;
   }
 
@@ -95,8 +96,8 @@ public:
     return api_error::not_supported;
   }
 
-  [[nodiscard]] auto write(std::uint64_t, const data_buffer &,
+  [[nodiscard]] auto write(std::uint64_t, const data_buffer &, std::size_t &)
-                           std::size_t &) -> api_error override {
+      -> api_error override {
     return api_error::not_supported;
   }
 };

repertory/librepertory/include/file_manager/ring_buffer_open_file.hpp

@@ -42,8 +42,8 @@ public:
   ring_buffer_open_file() = delete;
   ring_buffer_open_file(const ring_buffer_open_file &) noexcept = delete;
   ring_buffer_open_file(ring_buffer_open_file &&) noexcept = delete;
-  auto operator=(ring_buffer_open_file &&) noexcept -> ring_buffer_open_file & =
+  auto operator=(ring_buffer_open_file &&) noexcept
-                                                           delete;
+      -> ring_buffer_open_file & = delete;
   auto operator=(const ring_buffer_open_file &) noexcept
       -> ring_buffer_open_file & = delete;
 
@@ -57,6 +57,7 @@ private:
   std::condition_variable chunk_notify_;
   mutable std::mutex chunk_mtx_;
   std::mutex read_mtx_;
+  std::unique_ptr<std::thread> reader_thread_;
   std::size_t ring_begin_{};
   std::size_t ring_end_{};
   std::size_t ring_pos_{};
@@ -64,11 +65,9 @@ private:
   stop_type stop_requested_{false};
 
 private:
-  auto download_chunk(std::size_t chunk) -> api_error;
+  auto download_chunk(std::size_t chunk, bool skip_active) -> api_error;
 
-  void forward_reader_thread(std::size_t count);
+  void background_reader_thread();
-
-  void reverse_reader_thread(std::size_t count);
 
 protected:
   [[nodiscard]] auto is_download_complete() const -> bool override {
@@ -108,8 +107,8 @@ public:
     return false;
   }
 
-  [[nodiscard]] auto
+  [[nodiscard]] auto native_operation(native_operation_callback callback)
-  native_operation(native_operation_callback callback) -> api_error override;
+      -> api_error override;
 
   [[nodiscard]] auto native_operation(std::uint64_t, native_operation_callback)
       -> api_error override {
@@ -129,8 +128,8 @@ public:
 
   void set_api_path(const std::string &api_path) override;
 
-  [[nodiscard]] auto write(std::uint64_t, const data_buffer &,
+  [[nodiscard]] auto write(std::uint64_t, const data_buffer &, std::size_t &)
-                           std::size_t &) -> api_error override {
+      -> api_error override {
     return api_error::not_supported;
   }
 };

repertory/librepertory/include/types/repertory.hpp

@@ -194,6 +194,7 @@ enum class api_error {
   invalid_handle,
   invalid_operation,
   invalid_ring_buffer_multiple,
+  invalid_ring_buffer_position,
   invalid_ring_buffer_size,
   invalid_version,
   item_exists,
@@ -215,31 +216,33 @@ enum class api_error {
 
 [[nodiscard]] auto api_error_from_string(std::string_view str) -> api_error;
 
-[[nodiscard]] auto
+[[nodiscard]] auto api_error_to_string(const api_error &error)
-api_error_to_string(const api_error &error) -> const std::string &;
+    -> const std::string &;
 
 enum class database_type {
   rocksdb,
   sqlite,
 };
-[[nodiscard]] auto database_type_from_string(
-    std::string type,
-    database_type default_type = database_type::rocksdb) -> database_type;
-
 [[nodiscard]] auto
-database_type_to_string(const database_type &type) -> std::string;
+database_type_from_string(std::string type,
+                          database_type default_type = database_type::rocksdb)
+    -> database_type;
+
+[[nodiscard]] auto database_type_to_string(const database_type &type)
+    -> std::string;
 
 enum class download_type {
   direct,
   fallback,
   ring_buffer,
 };
-[[nodiscard]] auto download_type_from_string(
-    std::string type,
-    download_type default_type = download_type::fallback) -> download_type;
-
 [[nodiscard]] auto
-download_type_to_string(const download_type &type) -> std::string;
+download_type_from_string(std::string type,
+                          download_type default_type = download_type::fallback)
+    -> download_type;
+
+[[nodiscard]] auto download_type_to_string(const download_type &type)
+    -> std::string;
 
 enum class exit_code : std::int32_t {
   success = 0,

repertory/librepertory/src/file_manager/direct_open_file.cpp

@@ -21,10 +21,13 @@
 */
 #include "file_manager/direct_open_file.hpp"
 
+#include "events/event_system.hpp"
+#include "file_manager/events.hpp"
 #include "file_manager/open_file_base.hpp"
 #include "providers/i_provider.hpp"
 #include "types/repertory.hpp"
 #include "utils/common.hpp"
+#include "utils/time.hpp"
 
 namespace repertory {
 direct_open_file::direct_open_file(std::uint64_t chunk_size,
@@ -32,13 +35,20 @@ direct_open_file::direct_open_file(std::uint64_t chunk_size,
                                    filesystem_item fsi, i_provider &provider)
     : open_file_base(chunk_size, chunk_timeout, fsi, provider, true),
       total_chunks_(static_cast<std::size_t>(
-          utils::divide_with_ceiling(fsi.size, chunk_size))) {}
+          utils::divide_with_ceiling(fsi.size, chunk_size))) {
+  event_system::instance().raise<download_begin>(fsi_.api_path, "");
+}
 
 direct_open_file::~direct_open_file() { close(); }
 
 auto direct_open_file::close() -> bool {
   stop_requested_ = true;
-  return open_file_base::close();
+  last_progress_ = 0U;
+  auto ret = open_file_base::close();
+
+  event_system::instance().raise<download_end>(fsi_.api_path, "",
+                                               api_error::download_stopped);
+  return ret;
 }
 
 auto direct_open_file::read(std::size_t read_size, std::uint64_t read_offset,
@@ -56,7 +66,20 @@ auto direct_open_file::read(std::size_t read_size, std::uint64_t read_offset,
 
   auto res = provider_.read_file_bytes(fsi_.api_path, read_size, read_offset,
                                        data, stop_requested_);
+  if (res != api_error::success) {
+    return res;
+  }
+
   reset_timeout();
+  if ((utils::time::get_time_now() - last_progress_.load()) >
+      (2U * utils::time::NANOS_PER_SECOND)) {
+    last_progress_ = utils::time::get_time_now();
+    auto progress = (static_cast<double>(read_offset + read_size) /
+                     static_cast<double>(fsi_.size)) *
+                    100.0;
+    event_system::instance().raise<download_progress>(fsi_.api_path, "",
+                                                      progress);
+  }
 
   return res;
 }

repertory/librepertory/src/file_manager/file_manager.cpp

@@ -137,17 +137,21 @@ auto file_manager::create(const std::string &api_path, api_meta_map &meta,
 
 auto file_manager::evict_file(const std::string &api_path) -> bool {
   REPERTORY_USES_FUNCTION_NAME();
+  fmt::println("try evict|{}", api_path);
 
   if (provider_.is_read_only()) {
     return false;
   }
 
-  recur_mutex_lock open_lock(open_file_mtx_);
+  unique_recur_mutex_lock open_lock(open_file_mtx_);
+  fmt::println("try evict locked|{}", api_path);
   if (is_processing(api_path)) {
+    fmt::println("proccessing|{}", api_path);
     return false;
   }
 
   if (get_open_file_count(api_path) != 0U) {
+    fmt::println("open count|{}", api_path);
     return false;
   }
 
@@ -174,7 +178,14 @@ auto file_manager::evict_file(const std::string &api_path) -> bool {
     return false;
   }
 
+  std::shared_ptr<i_closeable_open_file> closeable_file;
+  if (open_file_lookup_.contains(api_path)) {
+    closeable_file = open_file_lookup_.at(api_path);
+  }
   open_file_lookup_.erase(api_path);
+  open_lock.unlock();
+
+  closeable_file.reset();
 
   auto file = utils::file::file{source_path};
   auto file_size = file.size().value_or(0U);
@@ -372,10 +383,11 @@ auto file_manager::open(const std::string &api_path, bool directory,
   return open(api_path, directory, ofd, handle, file, nullptr);
 }
 
-auto file_manager::open(
+auto file_manager::open(const std::string &api_path, bool directory,
-    const std::string &api_path, bool directory, const open_file_data &ofd,
+                        const open_file_data &ofd, std::uint64_t &handle,
-    std::uint64_t &handle, std::shared_ptr<i_open_file> &file,
+                        std::shared_ptr<i_open_file> &file,
-    std::shared_ptr<i_closeable_open_file> closeable_file) -> api_error {
+                        std::shared_ptr<i_closeable_open_file> closeable_file)
+    -> api_error {
   REPERTORY_USES_FUNCTION_NAME();
 
   const auto create_and_add_handle =
@@ -427,7 +439,7 @@ auto file_manager::open(
     auto ring_size{ring_buffer_file_size / chunk_size};
 
     const auto get_download_type = [&](download_type type) -> download_type {
-      if (fsi.size == 0U) {
+      if (directory || fsi.size == 0U) {
         return download_type::fallback;
       }
 
@@ -465,8 +477,10 @@ auto file_manager::open(
     };
 
     auto type = get_download_type(config_.get_preferred_download_type());
-    event_system::instance().raise<download_type_selected>(
+    if (not directory) {
-        fsi.api_path, fsi.source_path, type);
+      event_system::instance().raise<download_type_selected>(
+          fsi.api_path, fsi.source_path, type);
+    }
 
     switch (type) {
     case repertory::download_type::direct: {
@@ -568,7 +582,7 @@ auto file_manager::remove_file(const std::string &api_path) -> api_error {
 
 void file_manager::remove_resume(const std::string &api_path,
                                  const std::string &source_path) {
-  return remove_resume(api_path, source_path, false);
+  remove_resume(api_path, source_path, false);
 }
 
 void file_manager::remove_resume(const std::string &api_path,
@@ -711,8 +725,8 @@ auto file_manager::rename_directory(const std::string &from_api_path,
 }
 
 auto file_manager::rename_file(const std::string &from_api_path,
-                               const std::string &to_api_path,
+                               const std::string &to_api_path, bool overwrite)
-                               bool overwrite) -> api_error {
+    -> api_error {
   if (not provider_.is_rename_supported()) {
     return api_error::not_implemented;
   }
@@ -931,10 +945,10 @@ void file_manager::swap_renamed_items(std::string from_api_path,
 
   auto file_iter = open_file_lookup_.find(from_api_path);
   if (file_iter != open_file_lookup_.end()) {
-    auto ptr = std::move(open_file_lookup_[from_api_path]);
+    auto closeable_file = std::move(open_file_lookup_[from_api_path]);
     open_file_lookup_.erase(from_api_path);
-    ptr->set_api_path(to_api_path);
+    closeable_file->set_api_path(to_api_path);
-    open_file_lookup_[to_api_path] = std::move(ptr);
+    open_file_lookup_[to_api_path] = std::move(closeable_file);
   }
 
   if (directory) {

repertory/librepertory/src/file_manager/open_file.cpp

@@ -246,7 +246,8 @@ void open_file::download_chunk(std::size_t chunk, bool skip_active,
             read_state_.count(), get_api_error());
         if (get_api_error() == api_error::success) {
           auto progress = (static_cast<double>(read_state_.count()) /
-                           static_cast<double>(read_state_.size()) * 100.0);
+                           static_cast<double>(read_state_.size())) *
+                          100.0;
           event_system::instance().raise<download_progress>(
               fsi_.api_path, fsi_.source_path, progress);
           if (read_state_.all() && not notified_) {

repertory/librepertory/src/file_manager/ring_buffer_open_file.cpp

@@ -22,6 +22,7 @@
 #include "file_manager/ring_buffer_open_file.hpp"
 
 #include "app_config.hpp"
+#include "events/event_system.hpp"
 #include "file_manager/events.hpp"
 #include "file_manager/open_file_base.hpp"
 #include "platform/platform.hpp"
@@ -69,7 +70,7 @@ ring_buffer_open_file::ring_buffer_open_file(std::string buffer_directory,
   source_path_ =
       utils::path::combine(buffer_directory, {utils::create_uuid_string()});
   nf_ = utils::file::file::open_or_create_file(source_path_);
-  if (not *nf_) {
+  if (not*nf_) {
     throw std::runtime_error(fmt::format("failed to create buffer file|err|{}",
                                          utils::get_last_error_code()));
   }
@@ -79,6 +80,10 @@ ring_buffer_open_file::ring_buffer_open_file(std::string buffer_directory,
     throw std::runtime_error(fmt::format("failed to resize buffer file|err|{}",
                                          utils::get_last_error_code()));
   }
+
+  reader_thread_ =
+      std::make_unique<std::thread>([this]() { background_reader_thread(); });
+  event_system::instance().raise<download_begin>(fsi_.api_path, source_path_);
 }
 
 ring_buffer_open_file::~ring_buffer_open_file() {
@@ -92,6 +97,53 @@ ring_buffer_open_file::~ring_buffer_open_file() {
         function_name, fsi_.api_path, source_path_,
         utils::get_last_error_code(), "failed to delete file");
   }
+
+  reader_thread_->join();
+  reader_thread_.reset();
+}
+
+void ring_buffer_open_file::background_reader_thread() {
+  unique_mutex_lock chunk_lock(chunk_mtx_);
+  auto next_chunk = ring_pos_;
+  chunk_notify_.notify_all();
+  chunk_lock.unlock();
+
+  while (not stop_requested_) {
+    chunk_lock.lock();
+
+    next_chunk = next_chunk + 1U > ring_end_ ? ring_begin_ : next_chunk + 1U;
+    const auto check_and_wait = [this, &chunk_lock, &next_chunk]() {
+      if (stop_requested_) {
+        chunk_notify_.notify_all();
+        chunk_lock.unlock();
+        return;
+      }
+
+      if (get_read_state().all()) {
+        chunk_notify_.wait(chunk_lock);
+        next_chunk = ring_pos_;
+      }
+
+      chunk_notify_.notify_all();
+      chunk_lock.unlock();
+    };
+
+    if (not ring_state_[next_chunk % ring_state_.size()]) {
+      check_and_wait();
+      continue;
+    }
+
+    chunk_notify_.notify_all();
+    chunk_lock.unlock();
+
+    download_chunk(next_chunk, true);
+
+    chunk_lock.lock();
+    check_and_wait();
+  }
+
+  event_system::instance().raise<download_end>(fsi_.api_path, source_path_,
+                                               api_error::download_stopped);
 }
 
 auto ring_buffer_open_file::can_handle_file(std::uint64_t file_size,
@@ -102,60 +154,97 @@ auto ring_buffer_open_file::can_handle_file(std::uint64_t file_size,
 
 auto ring_buffer_open_file::close() -> bool {
   stop_requested_ = true;
+
+  unique_mutex_lock chunk_lock(chunk_mtx_);
+  chunk_notify_.notify_all();
+  chunk_lock.unlock();
+
   return open_file_base::close();
 }
 
-auto ring_buffer_open_file::download_chunk(std::size_t chunk) -> api_error {
+auto ring_buffer_open_file::download_chunk(std::size_t chunk, bool skip_active)
+    -> api_error {
   unique_mutex_lock chunk_lock(chunk_mtx_);
-  if (active_downloads_.find(chunk) != active_downloads_.end()) {
+  const auto unlock_and_notify = [this, &chunk_lock]() {
-    auto active_download = active_downloads_.at(chunk);
     chunk_notify_.notify_all();
     chunk_lock.unlock();
+  };
+
+  const auto unlock_and_return =
+      [&unlock_and_notify](api_error res) -> api_error {
+    unlock_and_notify();
+    return res;
+  };
+
+  if (chunk < ring_begin_ || chunk > ring_end_) {
+    return unlock_and_return(api_error::invalid_ring_buffer_position);
+  }
+
+  if (active_downloads_.find(chunk) != active_downloads_.end()) {
+    if (skip_active) {
+      return unlock_and_return(api_error::success);
+    }
+
+    auto active_download = active_downloads_.at(chunk);
+    unlock_and_notify();
 
     return active_download->wait();
   }
 
-  if (ring_state_[chunk % ring_state_.size()]) {
+  if (not ring_state_[chunk % ring_state_.size()]) {
-    auto active_download{std::make_shared<download>()};
+    return unlock_and_return(api_error::success);
-    active_downloads_[chunk] = active_download;
-    ring_state_[chunk % ring_state_.size()] = false;
-    chunk_notify_.notify_all();
-    chunk_lock.unlock();
-
-    data_buffer buffer;
-    auto data_offset{chunk * chunk_size_};
-    auto data_size{
-        chunk == (total_chunks_ - 1U) ? last_chunk_size_ : chunk_size_,
-    };
-
-    auto res{
-        provider_.read_file_bytes(fsi_.api_path, data_size, data_offset, buffer,
-                                  stop_requested_),
-    };
-    if (res == api_error::success) {
-      res = do_io([&]() -> api_error {
-        std::size_t bytes_written{};
-        if (nf_->write(buffer, (chunk % ring_state_.size()) * chunk_size_,
-                       &bytes_written)) {
-          return api_error::success;
-        }
-
-        return api_error::os_error;
-      });
-    }
-
-    active_download->notify(res);
-
-    chunk_lock.lock();
-    active_downloads_.erase(chunk);
-    chunk_notify_.notify_all();
-    return res;
   }
 
-  chunk_notify_.notify_all();
+  auto active_download{std::make_shared<download>()};
-  chunk_lock.unlock();
+  active_downloads_[chunk] = active_download;
+  ring_state_[chunk % ring_state_.size()] = false;
+  unlock_and_notify();
 
-  return api_error::success;
+  data_buffer buffer;
+  auto data_offset{chunk * chunk_size_};
+  auto data_size{
+      chunk == (total_chunks_ - 1U) ? last_chunk_size_ : chunk_size_,
+  };
+
+  event_system::instance().raise<download_chunk_begin>(
+      fsi_.api_path, source_path_, chunk, get_read_state().size(),
+      get_read_state().count());
+
+  auto res{
+      provider_.read_file_bytes(fsi_.api_path, data_size, data_offset, buffer,
+                                stop_requested_),
+  };
+
+  chunk_lock.lock();
+  if (res == api_error::success) {
+    auto progress =
+        (static_cast<double>(chunk + 1U) / static_cast<double>(total_chunks_)) *
+        100.0;
+    event_system::instance().raise<download_progress>(fsi_.api_path,
+                                                      source_path_, progress);
+    res = (chunk >= ring_begin_ && chunk <= ring_end_)
+              ? do_io([&]() -> api_error {
+                  std::size_t bytes_written{};
+                  if (nf_->write(buffer,
+                                 (chunk % ring_state_.size()) * chunk_size_,
+                                 &bytes_written)) {
+                    return api_error::success;
+                  }
+
+                  return api_error::os_error;
+                })
+              : api_error::invalid_ring_buffer_position;
+  }
+
+  event_system::instance().raise<download_chunk_end>(
+      fsi_.api_path, source_path_, chunk, get_read_state().size(),
+      get_read_state().count(), res);
+
+  active_downloads_.erase(chunk);
+  unlock_and_notify();
+
+  active_download->notify(res);
+  return res;
 }
 
 void ring_buffer_open_file::forward(std::size_t count) {
@@ -179,6 +268,7 @@ void ring_buffer_open_file::forward(std::size_t count) {
       ring_begin_ += added;
       ring_pos_ += count;
     }
+
     ring_end_ =
         std::min(total_chunks_ - 1U, ring_begin_ + ring_state_.size() - 1U);
   }
@@ -204,9 +294,7 @@ auto ring_buffer_open_file::native_operation(
 
 void ring_buffer_open_file::reverse(std::size_t count) {
   mutex_lock chunk_lock(chunk_mtx_);
-  if (ring_pos_ < count) {
+  count = std::min(ring_pos_, count);
-    count = ring_pos_;
-  }
 
   if ((ring_pos_ - count) >= ring_begin_) {
     ring_pos_ -= count;
@@ -232,16 +320,14 @@ void ring_buffer_open_file::reverse(std::size_t count) {
 }
 
 auto ring_buffer_open_file::read(std::size_t read_size,
-                                 std::uint64_t read_offset,
+                                 std::uint64_t read_offset, data_buffer &data)
-                                 data_buffer &data) -> api_error {
+    -> api_error {
   if (fsi_.directory) {
     return api_error::invalid_operation;
   }
 
   reset_timeout();
 
-  mutex_lock lock(read_mtx_);
-
   read_size = utils::calculate_read_size(fsi_.size, read_size, read_offset);
   if (read_size == 0U) {
     return api_error::success;
@@ -251,6 +337,8 @@ auto ring_buffer_open_file::read(std::size_t read_size,
   read_offset = read_offset - (begin_chunk * chunk_size_);
 
   auto res{api_error::success};
+
+  unique_mutex_lock read_lock(read_mtx_);
   for (std::size_t chunk = begin_chunk;
        (res == api_error::success) && (read_size > 0U); ++chunk) {
     if (chunk > ring_pos_) {
@@ -259,10 +347,16 @@ auto ring_buffer_open_file::read(std::size_t read_size,
       reverse(ring_pos_ - chunk);
     }
 
-    reset_timeout();
+    res = download_chunk(chunk, false);
-
-    res = download_chunk(chunk);
     if (res != api_error::success) {
+      if (not stop_requested_ &&
+          res == api_error::invalid_ring_buffer_position) {
+        read_lock.unlock();
+
+        // TODO limit retry
+        return read(read_size, read_offset, data);
+      }
+
       return res;
     }
 

repertory/librepertory/src/types/repertory.cpp

@@ -25,8 +25,8 @@
 #include "utils/string.hpp"
 
 namespace repertory {
-auto database_type_from_string(std::string type,
+auto database_type_from_string(std::string type, database_type default_type)
-                               database_type default_type) -> database_type {
+    -> database_type {
   type = utils::string::to_lower(utils::string::trim(type));
   if (type == "rocksdb") {
     return database_type::rocksdb;
@@ -50,8 +50,8 @@ auto database_type_to_string(const database_type &type) -> std::string {
   }
 }
 
-auto download_type_from_string(std::string type,
+auto download_type_from_string(std::string type, download_type default_type)
-                               download_type default_type) -> download_type {
+    -> download_type {
   type = utils::string::to_lower(utils::string::trim(type));
   if (type == "direct") {
     return download_type::direct;
@@ -106,6 +106,7 @@ static const std::unordered_map<api_error, std::string> LOOKUP = {
     {api_error::invalid_handle, "invalid_handle"},
     {api_error::invalid_operation, "invalid_operation"},
     {api_error::invalid_ring_buffer_multiple, "invalid_ring_buffer_multiple"},
+    {api_error::invalid_ring_buffer_position, "invalid_ring_buffer_position"},
     {api_error::invalid_ring_buffer_size, "invalid_ring_buffer_size"},
     {api_error::invalid_version, "invalid_version"},
     {api_error::item_exists, "item_exists"},

repertory/repertory_test/src/direct_open_file_test.cpp

@@ -29,12 +29,22 @@ constexpr const std::size_t test_chunk_size{1024U};
 } // namespace
 
 namespace repertory {
-TEST(direct_open_file, read_full_file) {
+class direct_open_file_test : public ::testing::Test {
+public:
+  console_consumer con_consumer;
+  mock_provider provider;
+
+protected:
+  void SetUp() override { event_system::instance().start(); }
+
+  void TearDown() override { event_system::instance().stop(); }
+};
+
+TEST_F(direct_open_file_test, read_full_file) {
   auto &source_file = test::create_random_file(test_chunk_size * 32U);
 
   auto dest_path = test::generate_test_file_name("direct_open_file");
 
-  mock_provider provider;
   EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
@@ -89,12 +99,11 @@ TEST(direct_open_file, read_full_file) {
   }
 }
 
-TEST(direct_open_file, read_full_file_in_reverse) {
+TEST_F(direct_open_file_test, read_full_file_in_reverse) {
   auto &source_file = test::create_random_file(test_chunk_size * 32U);
 
   auto dest_path = test::generate_test_file_name("direct_open_file");
 
-  mock_provider provider;
   EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
@@ -149,12 +158,11 @@ TEST(direct_open_file, read_full_file_in_reverse) {
   }
 }
 
-TEST(direct_open_file, read_full_file_in_partial_chunks) {
+TEST_F(direct_open_file_test, read_full_file_in_partial_chunks) {
   auto &source_file = test::create_random_file(test_chunk_size * 32U);
 
   auto dest_path = test::generate_test_file_name("test");
 
-  mock_provider provider;
   EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
@@ -206,12 +214,11 @@ TEST(direct_open_file, read_full_file_in_partial_chunks) {
   }
 }
 
-TEST(direct_open_file, read_full_file_in_partial_chunks_in_reverse) {
+TEST_F(direct_open_file_test, read_full_file_in_partial_chunks_in_reverse) {
   auto &source_file = test::create_random_file(test_chunk_size * 32U);
 
   auto dest_path = test::generate_test_file_name("direct_open_file");
 
-  mock_provider provider;
   EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;

repertory/repertory_test/src/open_file_test.cpp

@@ -30,9 +30,11 @@
 #include "utils/event_capture.hpp"
 #include "utils/path.hpp"
 
-namespace repertory {
+namespace {
-static constexpr const std::size_t test_chunk_size{1024U};
+constexpr const std::size_t test_chunk_size{1024U};
+}
 
+namespace repertory {
 class open_file_test : public ::testing::Test {
 public:
   console_consumer con_consumer;

repertory/repertory_test/src/ring_buffer_open_file_test.cpp

@@ -31,18 +31,29 @@
 namespace {
 constexpr const std::size_t test_chunk_size{1024U};
 
-std::string ring_buffer_dir = repertory::utils::path::combine(
+const auto ring_buffer_dir{
-    repertory::test::get_test_output_dir(),
+    repertory::utils::path::combine(
-    {"file_manager_ring_buffer_open_file_test"});
+        repertory::test::get_test_output_dir(),
+        {"file_manager_ring_buffer_open_file_test"}),
+};
 } // namespace
 
 namespace repertory {
-TEST(ring_buffer_open_file, can_forward_to_last_chunk) {
+class ring_buffer_open_file_test : public ::testing::Test {
+public:
+  console_consumer con_consumer;
+  mock_provider provider;
+
+protected:
+  void SetUp() override { event_system::instance().start(); }
+
+  void TearDown() override { event_system::instance().stop(); }
+};
+
+TEST_F(ring_buffer_open_file_test, can_forward_to_last_chunk) {
   auto source_path = test::generate_test_file_name("ring_buffer_open_file");
 
-  mock_provider prov;
+  EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
-
-  EXPECT_CALL(prov, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
   fsi.directory = false;
@@ -51,8 +62,8 @@ TEST(ring_buffer_open_file, can_forward_to_last_chunk) {
   fsi.source_path = source_path;
 
   {
-    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi, prov,
+    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi,
-                               8U);
+                               provider, 8U);
     file.set(0U, 3U);
     file.forward(4U);
 
@@ -63,17 +74,13 @@ TEST(ring_buffer_open_file, can_forward_to_last_chunk) {
       EXPECT_TRUE(file.get_read_state(chunk));
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file,
+TEST_F(ring_buffer_open_file_test,
-     can_forward_to_last_chunk_if_count_is_greater_than_remaining) {
+       can_forward_to_last_chunk_if_count_is_greater_than_remaining) {
   auto source_path = test::generate_test_file_name("ring_buffer_open_file");
 
-  mock_provider prov;
+  EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
-
-  EXPECT_CALL(prov, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
   fsi.directory = false;
@@ -82,8 +89,8 @@ TEST(ring_buffer_open_file,
   fsi.source_path = source_path;
 
   {
-    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi, prov,
+    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi,
-                               8U);
+                               provider, 8U);
     file.set(0U, 3U);
     file.forward(100U);
 
@@ -94,16 +101,12 @@ TEST(ring_buffer_open_file,
       EXPECT_FALSE(file.get_read_state(chunk));
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, can_forward_after_last_chunk) {
+TEST_F(ring_buffer_open_file_test, can_forward_after_last_chunk) {
   auto source_path = test::generate_test_file_name("ring_buffer_open_file");
 
-  mock_provider prov;
+  EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
-
-  EXPECT_CALL(prov, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
   fsi.directory = false;
@@ -112,8 +115,8 @@ TEST(ring_buffer_open_file, can_forward_after_last_chunk) {
   fsi.source_path = source_path;
 
   {
-    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi, prov,
+    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi,
-                               8U);
+                               provider, 8U);
     file.set(0U, 3U);
     file.forward(5U);
 
@@ -125,16 +128,12 @@ TEST(ring_buffer_open_file, can_forward_after_last_chunk) {
       EXPECT_TRUE(file.get_read_state(chunk));
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, can_forward_and_rollover_after_last_chunk) {
+TEST_F(ring_buffer_open_file_test, can_forward_and_rollover_after_last_chunk) {
   auto source_path = test::generate_test_file_name("ring_buffer_open_file");
 
-  mock_provider prov;
+  EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
-
-  EXPECT_CALL(prov, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
   fsi.directory = false;
@@ -143,8 +142,8 @@ TEST(ring_buffer_open_file, can_forward_and_rollover_after_last_chunk) {
   fsi.source_path = source_path;
 
   {
-    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi, prov,
+    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi,
-                               8U);
+                               provider, 8U);
     file.set(16U, 20U);
     file.forward(8U);
 
@@ -152,16 +151,12 @@ TEST(ring_buffer_open_file, can_forward_and_rollover_after_last_chunk) {
     EXPECT_EQ(std::size_t(21U), file.get_first_chunk());
     EXPECT_EQ(std::size_t(28U), file.get_last_chunk());
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, can_reverse_to_first_chunk) {
+TEST_F(ring_buffer_open_file_test, can_reverse_to_first_chunk) {
   auto source_path = test::generate_test_file_name("ring_buffer_open_file");
 
-  mock_provider prov;
+  EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
-
-  EXPECT_CALL(prov, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
   fsi.directory = false;
@@ -170,8 +165,8 @@ TEST(ring_buffer_open_file, can_reverse_to_first_chunk) {
   fsi.source_path = source_path;
 
   {
-    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi, prov,
+    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi,
-                               8U);
+                               provider, 8U);
     file.set(0U, 3U);
     file.reverse(3U);
 
@@ -182,17 +177,13 @@ TEST(ring_buffer_open_file, can_reverse_to_first_chunk) {
       EXPECT_TRUE(file.get_read_state(chunk));
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file,
+TEST_F(ring_buffer_open_file_test,
-     can_reverse_to_first_chunk_if_count_is_greater_than_remaining) {
+       can_reverse_to_first_chunk_if_count_is_greater_than_remaining) {
   auto source_path = test::generate_test_file_name("ring_buffer_open_file");
 
-  mock_provider prov;
+  EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
-
-  EXPECT_CALL(prov, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
   fsi.directory = false;
@@ -201,8 +192,8 @@ TEST(ring_buffer_open_file,
   fsi.source_path = source_path;
 
   {
-    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi, prov,
+    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi,
-                               8U);
+                               provider, 8U);
     file.set(0U, 3U);
     file.reverse(13U);
 
@@ -213,16 +204,12 @@ TEST(ring_buffer_open_file,
       EXPECT_TRUE(file.get_read_state(chunk));
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, can_reverse_before_first_chunk) {
+TEST_F(ring_buffer_open_file_test, can_reverse_before_first_chunk) {
   auto source_path = test::generate_test_file_name("ring_buffer_open_file");
 
-  mock_provider prov;
+  EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
-
-  EXPECT_CALL(prov, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
   fsi.directory = false;
@@ -231,8 +218,8 @@ TEST(ring_buffer_open_file, can_reverse_before_first_chunk) {
   fsi.source_path = source_path;
 
   {
-    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi, prov,
+    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi,
-                               8U);
+                               provider, 8U);
     file.set(1U, 3U);
     file.reverse(3U);
 
@@ -244,16 +231,13 @@ TEST(ring_buffer_open_file, can_reverse_before_first_chunk) {
       EXPECT_TRUE(file.get_read_state(chunk));
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, can_reverse_and_rollover_before_first_chunk) {
+TEST_F(ring_buffer_open_file_test,
+       can_reverse_and_rollover_before_first_chunk) {
   auto source_path = test::generate_test_file_name("ring_buffer_open_file");
 
-  mock_provider prov;
+  EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
-
-  EXPECT_CALL(prov, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
   fsi.directory = false;
@@ -262,8 +246,8 @@ TEST(ring_buffer_open_file, can_reverse_and_rollover_before_first_chunk) {
   fsi.source_path = source_path;
 
   {
-    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi, prov,
+    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi,
-                               8U);
+                               provider, 8U);
     file.set(16U, 20U);
     file.reverse(8U);
 
@@ -279,16 +263,12 @@ TEST(ring_buffer_open_file, can_reverse_and_rollover_before_first_chunk) {
       EXPECT_TRUE(file.get_read_state(chunk));
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, can_reverse_full_ring) {
+TEST_F(ring_buffer_open_file_test, can_reverse_full_ring) {
   auto source_path = test::generate_test_file_name("ring_buffer_open_file");
 
-  mock_provider prov;
+  EXPECT_CALL(provider, is_read_only()).WillRepeatedly(Return(false));
-
-  EXPECT_CALL(prov, is_read_only()).WillRepeatedly(Return(false));
 
   filesystem_item fsi;
   fsi.directory = false;
@@ -297,8 +277,8 @@ TEST(ring_buffer_open_file, can_reverse_full_ring) {
   fsi.source_path = source_path;
 
   {
-    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi, prov,
+    ring_buffer_open_file file(ring_buffer_dir, test_chunk_size, 30U, fsi,
-                               8U);
+                               provider, 8U);
     file.set(8U, 15U);
     file.reverse(16U);
 
@@ -310,11 +290,9 @@ TEST(ring_buffer_open_file, can_reverse_full_ring) {
       EXPECT_FALSE(file.get_read_state(chunk));
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, read_full_file) {
+TEST_F(ring_buffer_open_file_test, read_full_file) {
   auto &nf = test::create_random_file(test_chunk_size * 33u + 11u);
   auto download_source_path = nf.get_path();
 
@@ -375,11 +353,9 @@ TEST(ring_buffer_open_file, read_full_file) {
       EXPECT_STREQ(hash1.value().c_str(), hash2.value().c_str());
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, read_full_file_in_reverse) {
+TEST_F(ring_buffer_open_file_test, read_full_file_in_reverse) {
   auto &nf = test::create_random_file(test_chunk_size * 32u);
   auto download_source_path = nf.get_path();
 
@@ -417,15 +393,15 @@ TEST(ring_buffer_open_file, read_full_file_in_reverse) {
     EXPECT_TRUE(nf2);
 
     auto to_read = fsi.size;
-    std::size_t chunk = rb.get_total_chunks() - 1u;
+    std::size_t chunk = rb.get_total_chunks() - 1U;
-    while (to_read) {
+    while (to_read > 0U) {
       data_buffer data{};
       EXPECT_EQ(api_error::success,
                 rb.read(test_chunk_size, chunk * test_chunk_size, data));
 
       std::size_t bytes_written{};
       EXPECT_TRUE(nf2.write(data, chunk * test_chunk_size, &bytes_written));
-      chunk--;
+      --chunk;
       to_read -= data.size();
     }
     nf2.close();
@@ -440,11 +416,9 @@ TEST(ring_buffer_open_file, read_full_file_in_reverse) {
       EXPECT_STREQ(hash1.value().c_str(), hash2.value().c_str());
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, read_full_file_in_partial_chunks) {
+TEST_F(ring_buffer_open_file_test, read_full_file_in_partial_chunks) {
   auto &nf = test::create_random_file(test_chunk_size * 32u);
   auto download_source_path = nf.get_path();
 
@@ -506,11 +480,10 @@ TEST(ring_buffer_open_file, read_full_file_in_partial_chunks) {
       EXPECT_STREQ(hash1.value().c_str(), hash2.value().c_str());
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 
-TEST(ring_buffer_open_file, read_full_file_in_partial_chunks_in_reverse) {
+TEST_F(ring_buffer_open_file_test,
+       read_full_file_in_partial_chunks_in_reverse) {
   auto &nf = test::create_random_file(test_chunk_size * 32u);
   auto download_source_path = nf.get_path();
 
@@ -577,7 +550,5 @@ TEST(ring_buffer_open_file, read_full_file_in_partial_chunks_in_reverse) {
       EXPECT_STREQ(hash1.value().c_str(), hash2.value().c_str());
     }
   }
-
-  EXPECT_TRUE(utils::file::directory(ring_buffer_dir).remove_recursively());
 }
 } // namespace repertory