From ae0059591c7f5e58da4e077aabc784dca0103945 Mon Sep 17 00:00:00 2001 From: "Scott E. Graves" Date: Sat, 27 Jul 2024 08:12:43 -0500 Subject: [PATCH] fix directory caching --- .../drives/remote/remote_open_file_table.hpp | 9 ++- .../drives/remote/remote_server_base.hpp | 77 +++++++++---------- .../winfsp/remotewinfsp/remote_server.hpp | 7 ++ .../drives/fuse/remotefuse/remote_server.cpp | 7 +- .../drives/remote/remote_open_file_table.cpp | 21 ++--- .../winfsp/remotewinfsp/remote_server.cpp | 58 +++++++++----- 6 files changed, 100 insertions(+), 79 deletions(-) diff --git a/repertory/librepertory/include/drives/remote/remote_open_file_table.hpp b/repertory/librepertory/include/drives/remote/remote_open_file_table.hpp index 6d2d7b50..bc45d649 100644 --- a/repertory/librepertory/include/drives/remote/remote_open_file_table.hpp +++ b/repertory/librepertory/include/drives/remote/remote_open_file_table.hpp @@ -49,13 +49,13 @@ protected: private: std::unordered_map compat_lookup_; std::recursive_mutex compat_mutex_; - std::unordered_map> directory_lookup_; + std::unordered_map> directory_lookup_; std::recursive_mutex directory_mutex_; std::unordered_map file_lookup_; mutable std::recursive_mutex file_mutex_; protected: - void add_directory(const std::string &client_id, void *dir); + void add_directory(const std::string &client_id, std::uint64_t handle); void close_all(const std::string &client_id); @@ -71,7 +71,7 @@ protected: open_info &oi) -> bool; [[nodiscard]] auto has_open_directory(const std::string &client_id, - void *dir) -> bool; + std::uint64_t handle) -> bool; [[nodiscard]] auto has_compat_open_info(const remote::file_handle &handle, int error_return) -> int; @@ -89,7 +89,8 @@ protected: void remove_compat_open_info(const remote::file_handle &handle); - auto remove_directory(const std::string &client_id, void *dir) -> bool; + auto remove_directory(const std::string &client_id, + std::uint64_t handle) -> bool; void remove_open_info(const native_handle &handle); diff --git a/repertory/librepertory/include/drives/remote/remote_server_base.hpp b/repertory/librepertory/include/drives/remote/remote_server_base.hpp index 1a577935..23817d4a 100644 --- a/repertory/librepertory/include/drives/remote/remote_server_base.hpp +++ b/repertory/librepertory/include/drives/remote/remote_server_base.hpp @@ -335,7 +335,7 @@ public: DECODE_OR_RETURN(request, length); data_buffer buffer(length); - UINT32 bytes_transferred = 0; + UINT32 bytes_transferred{0}; ret = this->winfsp_read(file_desc, buffer.data(), offset, length, &bytes_transferred); if (ret == STATUS_SUCCESS) { @@ -490,7 +490,7 @@ public: auto *buffer = request->current_pointer(); - UINT32 bytes_transferred = 0; + UINT32 bytes_transferred{0}; remote::file_info file_info{}; ret = this->winfsp_write(file_desc, buffer, offset, length, write_to_end, constrained_io, @@ -506,7 +506,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -521,7 +521,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -536,7 +536,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -551,7 +551,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -569,7 +569,7 @@ public: [this](std::uint32_t, const std::string &client_id, std::uint64_t, const std::string &, packet *request, packet &response) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -601,7 +601,7 @@ public: /*handlerLookup_.insert({"::fuse_fallocate", [this](std::uint32_t serviceFlags, const std::string &client_id, std::uint64_t threadId, const std::string &method, packet - *request, packet &response) -> packet::error_type { auto ret = 0; + *request, packet &response) -> packet::error_type { auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -626,7 +626,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &response) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -657,7 +657,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -675,7 +675,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -693,7 +693,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -711,7 +711,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &response) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -736,7 +736,7 @@ public: /*handlerLookup_.insert({"::fuse_getxattr", [this](std::uint32_t serviceFlags, const std::string &client_id, std::uint64_t threadId, const std::string &method, packet - *request, packet &response) -> packet::error_type { auto ret = 0; + *request, packet &response) -> packet::error_type { auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -753,7 +753,7 @@ public: handlerLookup_.insert({"::fuse_getxattr_osx", [this](std::uint32_t serviceFlags, const std::string &client_id, std::uint64_t threadId, const std::string &method, packet - *request, packet &response) -> packet::error_type { auto ret = 0; + *request, packet &response) -> packet::error_type { auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -775,7 +775,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &response) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -798,7 +798,7 @@ public: /*handlerLookup_.insert({"::remote_fuseListxattr", [this](std::uint32_t serviceFlags, const std::string &client_id, std::uint64_t threadId, const std::string &method, packet - *request, packet &response) -> packet::error_type { auto ret = 0; + *request, packet &response) -> packet::error_type { auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -814,7 +814,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -829,7 +829,7 @@ public: [this](std::uint32_t, const std::string &client_id, std::uint64_t, const std::string &, packet *request, packet &response) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -853,14 +853,14 @@ public: [this](std::uint32_t, const std::string &client_id, std::uint64_t, const std::string &, packet *request, packet &response) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); - remote::file_handle handle = 0; + remote::file_handle handle{0}; if ((ret = this->fuse_opendir(path.c_str(), handle)) >= 0) { - this->add_directory(client_id, reinterpret_cast(handle)); + this->add_directory(client_id, handle); response.encode(handle); } return ret; @@ -870,7 +870,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &response) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -896,7 +896,7 @@ public: [this](std::uint32_t, const std::string &client_id, std::uint64_t, const std::string &, packet *request, packet &response) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -907,8 +907,7 @@ public: remote::file_handle handle; DECODE_OR_RETURN(request, handle); - if (this->has_open_directory(client_id, - reinterpret_cast(handle))) { + if (this->has_open_directory(client_id, handle)) { std::string file_path; ret = this->fuse_readdir(path.c_str(), offset, handle, file_path); if (ret == 0) { @@ -925,7 +924,7 @@ public: [this](std::uint32_t, const std::string &, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -940,7 +939,7 @@ public: [this](std::uint32_t, const std::string &client_id, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -949,8 +948,7 @@ public: DECODE_OR_RETURN(request, handle); ret = this->fuse_releasedir(path.c_str(), handle); - if (this->remove_directory(client_id, - reinterpret_cast(handle))) { + if (this->remove_directory(client_id, handle)) { return ret; } return -EBADF; @@ -958,7 +956,7 @@ public: /*handlerLookup_.insert({"::fuse_removexattr", [this](std::uint32_t serviceFlags, const std::string &client_id, std::uint64_t threadId, const std::string &method, packet - *request, packet &response) -> packet::error_type { auto ret = 0; + *request, packet &response) -> packet::error_type { auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -1071,7 +1069,7 @@ public: /*handlerLookup_.insert({"::fuse_setxattr", [this](std::uint32_t serviceFlags, const std::string &client_id, std::uint64_t threadId, const std::string &method, packet - *request, packet &response) -> packet::error_type { auto ret = 0; + *request, packet &response) -> packet::error_type { auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -1100,7 +1098,7 @@ public: handlerLookup_.insert({"::fuse_setxattr_osx", [this](std::uint32_t serviceFlags, const std::string &client_id, std::uint64_t threadId, const std::string &method, packet - *request, packet &response) -> packet::error_type { auto ret = 0; + *request, packet &response) -> packet::error_type { auto ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -1300,8 +1298,7 @@ public: if ((ret = this->json_create_directory_snapshot(path.data(), json_data)) == 0) { this->add_directory(client_id, - reinterpret_cast( - json_data["handle"].get())); + json_data["handle"].get()); response.encode(json_data.dump(0)); } return ret; @@ -1311,7 +1308,7 @@ public: [this](std::uint32_t, const std::string &client_id, std::uint64_t, const std::string &, packet *request, packet &response) -> packet::error_type { - std::int32_t ret = 0; + std::int32_t ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -1323,8 +1320,7 @@ public: DECODE_OR_RETURN(request, page); ret = -EBADF; - if (this->has_open_directory(client_id, - reinterpret_cast(handle))) { + if (this->has_open_directory(client_id, handle)) { json json_data; json_data["directory_list"] = std::vector(); json_data["page"] = page; @@ -1342,7 +1338,7 @@ public: [this](std::uint32_t, const std::string &client_id, std::uint64_t, const std::string &, packet *request, packet &) -> packet::error_type { - auto ret = 0; + std::uint32_t ret{0}; std::string path; DECODE_OR_RETURN(request, path); @@ -1351,8 +1347,7 @@ public: DECODE_OR_RETURN(request, handle); ret = this->json_release_directory_snapshot(path.data(), handle); - if (this->remove_directory(client_id, - reinterpret_cast(handle))) { + if (this->remove_directory(client_id, handle)) { return ret; } return -EBADF; diff --git a/repertory/librepertory/include/drives/winfsp/remotewinfsp/remote_server.hpp b/repertory/librepertory/include/drives/winfsp/remotewinfsp/remote_server.hpp index 728f5b5c..d57cba14 100644 --- a/repertory/librepertory/include/drives/winfsp/remotewinfsp/remote_server.hpp +++ b/repertory/librepertory/include/drives/winfsp/remotewinfsp/remote_server.hpp @@ -26,6 +26,7 @@ #if defined(_WIN32) #include "comm/packet/packet.hpp" +#include "drives/directory_cache.hpp" #include "drives/remote/remote_server_base.hpp" #include "drives/winfsp/i_winfsp_drive.hpp" @@ -39,9 +40,15 @@ public: : remote_server_base(config, drive, utils::string::to_lower(mount_location)) {} +private: + directory_cache directory_cache_; + std::atomic next_handle_{0U}; + private: [[nodiscard]] auto construct_path(std::string path) -> std::string; + [[nodiscard]] auto get_next_handle() -> std::uint64_t; + [[nodiscard]] auto populate_file_info(const std::string &api_path, remote::file_info &file_info) -> packet::error_type; diff --git a/repertory/librepertory/src/drives/fuse/remotefuse/remote_server.cpp b/repertory/librepertory/src/drives/fuse/remotefuse/remote_server.cpp index 8a36c461..441ec3dc 100644 --- a/repertory/librepertory/src/drives/fuse/remotefuse/remote_server.cpp +++ b/repertory/librepertory/src/drives/fuse/remotefuse/remote_server.cpp @@ -685,11 +685,11 @@ auto remote_server::fuse_readdir(const char *path, res = -1; } else { auto iter = directory_cache_.get_directory(handle); - if (iter != nullptr) { - res = iter->get(static_cast(offset), item_path); - } else { + if (iter == nullptr) { res = -1; errno = EFAULT; + } else { + res = iter->get(static_cast(offset), item_path); } } @@ -1609,6 +1609,7 @@ auto remote_server::json_read_directory_snapshot( json_data["page_count"] = utils::divide_with_ceiling( iter->get_count(), REPERTORY_DIRECTORY_PAGE_SIZE); } + auto ret = ((res < 0) ? -errno : 0); RAISE_REMOTE_FUSE_SERVER_EVENT(function_name, file_path, ret); return ret; diff --git a/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp b/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp index 3e70795c..489269cc 100644 --- a/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp +++ b/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp @@ -27,11 +27,11 @@ namespace repertory { void remote_open_file_table::add_directory(const std::string &client_id, - void *dir) { + std::uint64_t handle) { recur_mutex_lock directory_lock(directory_mutex_); auto &list = directory_lookup_[client_id]; - if (utils::collection_excludes(list, dir)) { - directory_lookup_[client_id].emplace_back(dir); + if (utils::collection_excludes(list, handle)) { + directory_lookup_[client_id].emplace_back(handle); } } @@ -72,7 +72,7 @@ void remote_open_file_table::close_all(const std::string &client_id) { remove_open_info(handle); } - std::vector dirs; + std::vector dirs; unique_recur_mutex_lock directory_lock(directory_mutex_); for (auto &kv : directory_lookup_) { if (kv.first == client_id) { @@ -81,7 +81,7 @@ void remote_open_file_table::close_all(const std::string &client_id) { } directory_lock.unlock(); - for (auto *dir : dirs) { + for (auto &&dir : dirs) { remove_directory(client_id, dir); } } @@ -141,10 +141,10 @@ auto remote_open_file_table::get_open_file_path(const native_handle &handle) } auto remote_open_file_table::has_open_directory(const std::string &client_id, - void *dir) -> bool { + std::uint64_t handle) -> bool { recur_mutex_lock directory_lock(directory_mutex_); auto &list = directory_lookup_[client_id]; - return (utils::collection_includes(list, dir)); + return (utils::collection_includes(list, handle)); } auto remote_open_file_table::has_compat_open_info( @@ -204,16 +204,17 @@ void remote_open_file_table::remove_compat_open_info( } auto remote_open_file_table::remove_directory(const std::string &client_id, - void *dir) -> bool { + std::uint64_t handle) -> bool { recur_mutex_lock directory_lock(directory_mutex_); auto &list = directory_lookup_[client_id]; - if (utils::collection_includes(list, dir)) { - utils::remove_element_from(list, dir); + if (utils::collection_includes(list, handle)) { + utils::remove_element_from(list, handle); if (directory_lookup_[client_id].empty()) { directory_lookup_.erase(client_id); } return true; } + return false; } diff --git a/repertory/librepertory/src/drives/winfsp/remotewinfsp/remote_server.cpp b/repertory/librepertory/src/drives/winfsp/remotewinfsp/remote_server.cpp index 7a0431b3..cc8c8eb9 100644 --- a/repertory/librepertory/src/drives/winfsp/remotewinfsp/remote_server.cpp +++ b/repertory/librepertory/src/drives/winfsp/remotewinfsp/remote_server.cpp @@ -63,6 +63,14 @@ E_SIMPLE3(remote_winfsp_server_event, debug, true, ); // clang-format on +auto remote_server::get_next_handle() -> std::uint64_t { + if (++next_handle_ == 0U) { + return ++next_handle_; + } + + return next_handle_; +} + auto remote_server::construct_path(std::string path) -> std::string { path = utils::path::combine(mount_location_, {path}); if (path == mount_location_) { @@ -331,8 +339,11 @@ auto remote_server::fuse_opendir(const char *path, remote::file_handle &handle) if (::PathIsDirectoryW(unicode_file_path.c_str()) != 0) { auto list = drive_.get_directory_items(utils::path::create_api_path(path)); - handle = static_cast(reinterpret_cast( - new directory_iterator(std::move(list)))); + auto iter = std::make_shared(std::move(list)); + + handle = get_next_handle(); + directory_cache_.set_directory(path, handle, iter); + res = 0; errno = 0; } @@ -531,7 +542,7 @@ auto remote_server::fuse_readdir(const char *path, errno = ERANGE; res = -1; } else { - auto *iter = reinterpret_cast(handle); + auto iter = directory_cache_.get_directory(handle); if (iter == nullptr) { res = -1; errno = EFAULT; @@ -817,10 +828,12 @@ auto remote_server::json_create_directory_snapshot( if (utils::file::is_directory(file_path)) { auto list = drive_.get_directory_items(utils::path::create_api_path(path)); - auto *iter = new directory_iterator(std::move(list)); + auto iter = std::make_shared(std::move(list)); + auto handle = get_next_handle(); + directory_cache_.set_directory(api_path, handle, iter); + json_data["path"] = path; - json_data["handle"] = static_cast( - reinterpret_cast(iter)); + json_data["handle"] = handle; json_data["page_count"] = utils::divide_with_ceiling( iter->get_count(), REPERTORY_DIRECTORY_PAGE_SIZE); res = 0; @@ -838,22 +851,25 @@ auto remote_server::json_read_directory_snapshot( constexpr const auto *function_name = static_cast(__FUNCTION__); const auto file_path = construct_path(path); - auto *iter = reinterpret_cast(handle); - std::size_t offset{}; - int res{}; - json item_json; - while ( - (json_data["directory_list"].size() < REPERTORY_DIRECTORY_PAGE_SIZE) && - (res = iter->get_json((page * REPERTORY_DIRECTORY_PAGE_SIZE) + offset++, - item_json)) == 0) { - json_data["directory_list"].emplace_back(item_json); + + int res{-EBADF}; + auto iter = directory_cache_.get_directory(handle); + if (iter != nullptr) { + std::size_t offset{}; + json item_json; + while ( + (json_data["directory_list"].size() < REPERTORY_DIRECTORY_PAGE_SIZE) && + (res = iter->get_json((page * REPERTORY_DIRECTORY_PAGE_SIZE) + offset++, + item_json)) == 0) { + json_data["directory_list"].emplace_back(item_json); + } + json_data["handle"] = handle; + json_data["path"] = path; + json_data["page"] = page; + json_data["page_count"] = utils::divide_with_ceiling( + iter->get_count(), REPERTORY_DIRECTORY_PAGE_SIZE); } - json_data["handle"] = - static_cast(reinterpret_cast(iter)); - json_data["path"] = path; - json_data["page"] = page; - json_data["page_count"] = utils::divide_with_ceiling( - iter->get_count(), REPERTORY_DIRECTORY_PAGE_SIZE); + const auto ret = ((res < 0) ? -errno : 0); RAISE_REMOTE_WINFSP_SERVER_EVENT(function_name, file_path, ret); return ret;