From 8655becf1ea99bafe536ae3780a53bd2042143cd Mon Sep 17 00:00:00 2001 From: "Scott E. Graves" Date: Sat, 27 Jul 2024 07:54:46 -0500 Subject: [PATCH] fix directory caching --- .../include/drives/directory_cache.hpp | 13 ++-- .../drives/fuse/remotefuse/remote_server.hpp | 6 +- .../drives/remote/remote_open_file_table.hpp | 2 - .../drives/remote/remote_server_base.hpp | 65 +++++++++---------- .../winfsp/remotewinfsp/remote_client.hpp | 3 - .../src/drives/directory_cache.cpp | 53 ++++++++------- .../src/drives/fuse/fuse_drive.cpp | 12 ++-- .../drives/fuse/remotefuse/remote_server.cpp | 63 +++++++++--------- .../drives/remote/remote_open_file_table.cpp | 1 - 9 files changed, 105 insertions(+), 113 deletions(-) diff --git a/repertory/librepertory/include/drives/directory_cache.hpp b/repertory/librepertory/include/drives/directory_cache.hpp index fee5409b..9913fe5e 100644 --- a/repertory/librepertory/include/drives/directory_cache.hpp +++ b/repertory/librepertory/include/drives/directory_cache.hpp @@ -33,8 +33,9 @@ public: using execute_callback = std::function; private: - struct open_directory { - std::shared_ptr iterator{}; + struct open_directory final { + std::shared_ptr iterator; + std::vector handles{}; std::chrono::system_clock::time_point last_update{ std::chrono::system_clock::now()}; }; @@ -60,15 +61,15 @@ public: void execute_action(const std::string &api_path, const execute_callback &execute); - [[nodiscard]] auto get_directory(directory_iterator *iterator) - -> std::shared_ptr; + [[nodiscard]] auto + get_directory(std::uint64_t handle) -> std::shared_ptr; [[nodiscard]] auto remove_directory(const std::string &api_path) -> std::shared_ptr; - void remove_directory(directory_iterator *iterator); + void remove_directory(std::uint64_t handle); - void set_directory(const std::string &api_path, + void set_directory(const std::string &api_path, std::uint64_t handle, std::shared_ptr iterator); }; } // namespace repertory diff --git a/repertory/librepertory/include/drives/fuse/remotefuse/remote_server.hpp b/repertory/librepertory/include/drives/fuse/remotefuse/remote_server.hpp index bc8e75f0..37798603 100644 --- a/repertory/librepertory/include/drives/fuse/remotefuse/remote_server.hpp +++ b/repertory/librepertory/include/drives/fuse/remotefuse/remote_server.hpp @@ -38,6 +38,7 @@ public: private: directory_cache directory_cache_; + std::atomic next_handle_{0U}; private: [[nodiscard]] auto construct_path(std::string path) -> std::string; @@ -46,6 +47,8 @@ private: [[nodiscard]] static auto empty_as_zero(const json &data) -> 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; @@ -58,9 +61,6 @@ private: [[nodiscard]] auto update_to_windows_format(json &item) -> json &; -protected: - void delete_open_directory(void *dir) override; - public: // FUSE Layer [[nodiscard]] auto fuse_access(const char *path, const std::int32_t &mask) 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 f87be7af..6d2d7b50 100644 --- a/repertory/librepertory/include/drives/remote/remote_open_file_table.hpp +++ b/repertory/librepertory/include/drives/remote/remote_open_file_table.hpp @@ -59,8 +59,6 @@ protected: void close_all(const std::string &client_id); - virtual void delete_open_directory(void *dir) = 0; - #if defined(_WIN32) [[nodiscard]] auto get_directory_buffer(const native_handle &handle, PVOID *&buffer) -> bool; diff --git a/repertory/librepertory/include/drives/remote/remote_server_base.hpp b/repertory/librepertory/include/drives/remote/remote_server_base.hpp index d11bba41..1a577935 100644 --- a/repertory/librepertory/include/drives/remote/remote_server_base.hpp +++ b/repertory/librepertory/include/drives/remote/remote_server_base.hpp @@ -26,9 +26,7 @@ #include "comm/packet/client_pool.hpp" #include "comm/packet/packet.hpp" #include "comm/packet/packet_server.hpp" -#include "drives/directory_iterator.hpp" #include "drives/fuse/remotefuse/i_remote_instance.hpp" -#include "drives/remote/i_remote_json.hpp" #include "drives/remote/remote_open_file_table.hpp" #include "drives/winfsp/remotewinfsp/i_remote_instance.hpp" #include "types/remote.hpp" @@ -546,7 +544,7 @@ public: remote::file_mode mode; DECODE_OR_RETURN(request, mode); - return this->fuse_chmod(&path[0], mode); + return this->fuse_chmod(path.c_str(), mode); }}); handler_lookup_.insert( {"::fuse_chown", @@ -564,7 +562,7 @@ public: remote::group_id gid{}; DECODE_OR_RETURN(request, gid); - return this->fuse_chown(&path[0], uid, gid); + return this->fuse_chown(path.c_str(), uid, gid); }}); handler_lookup_.insert( {"::fuse_create", @@ -620,8 +618,8 @@ public: remote::file_handle handle; DECODE_OR_RETURN(request, handle); - return this->fuse_fallocate(&path[0], mode, offset, - length, handle); + return this->fuse_fallocate(path.c_str(), mode, + offset, length, handle); }});*/ handler_lookup_.insert( {"::fuse_fgetattr", @@ -688,7 +686,7 @@ public: remote::file_handle handle; DECODE_OR_RETURN(request, handle); - return this->fuse_fsync(&path[0], dataSync, handle); + return this->fuse_fsync(path.c_str(), dataSync, handle); }}); handler_lookup_.insert( {"::fuse_ftruncate", @@ -706,7 +704,7 @@ public: remote::file_handle handle; DECODE_OR_RETURN(request, handle); - return this->fuse_ftruncate(&path[0], size, handle); + return this->fuse_ftruncate(path.c_str(), size, handle); }}); handler_lookup_.insert( {"::fuse_getattr", @@ -726,7 +724,7 @@ public: remote::stat st{}; bool directory = false; - ret = this->fuse_getattr(&path[0], st, directory); + ret = this->fuse_getattr(path.c_str(), st, directory); if (ret == 0) { st.st_uid = uid; st.st_gid = gid; @@ -749,7 +747,7 @@ public: remote::file_size size; DECODE_OR_RETURN(request, size); - return this->fuse_getxattr(&path[0], &name[0], + return this->fuse_getxattr(path.c_str(), &name[0], nullptr, size); }}); handlerLookup_.insert({"::fuse_getxattr_osx", @@ -769,8 +767,8 @@ public: std::uint32_t position; DECODE_OR_RETURN(request, position); - return this->fuse_getxattr_osx(&path[0], &name[0], - nullptr, size, position); + return this->fuse_getxattr_osx(path.c_str(), + &name[0], nullptr, size, position); }});*/ handler_lookup_.insert( {"::fuse_getxtimes", @@ -784,7 +782,8 @@ public: remote::file_time bkuptime{}; remote::file_time crtime{}; - if ((ret = this->fuse_getxtimes(&path[0], bkuptime, crtime)) == 0) { + if ((ret = this->fuse_getxtimes(path.c_str(), bkuptime, crtime)) == + 0) { response.encode(bkuptime); response.encode(crtime); } @@ -807,7 +806,7 @@ public: remote::file_size size; DECODE_OR_RETURN(request, size); - return this->fuse_listxattr(&path[0], nullptr, + return this->fuse_listxattr(path.c_str(), nullptr, size); }});*/ handler_lookup_.insert( @@ -823,7 +822,7 @@ public: remote::file_mode mode; DECODE_OR_RETURN(request, mode); - return this->fuse_mkdir(&path[0], mode); + return this->fuse_mkdir(path.c_str(), mode); }}); handler_lookup_.insert( {"::fuse_open", @@ -839,7 +838,7 @@ public: DECODE_OR_RETURN(request, flags); remote::file_handle handle; - if ((ret = this->fuse_open(&path[0], flags, handle)) >= 0) { + if ((ret = this->fuse_open(path.c_str(), flags, handle)) >= 0) { #if defined(_WIN32) this->set_compat_client_id(handle, client_id); #else @@ -860,7 +859,7 @@ public: DECODE_OR_RETURN(request, path); remote::file_handle handle = 0; - if ((ret = this->fuse_opendir(&path[0], handle)) >= 0) { + if ((ret = this->fuse_opendir(path.c_str(), handle)) >= 0) { this->add_directory(client_id, reinterpret_cast(handle)); response.encode(handle); } @@ -886,10 +885,9 @@ public: DECODE_OR_RETURN(request, handle); data_buffer buffer; - if ((ret = - this->fuse_read(&path[0], reinterpret_cast(&buffer), - read_size, read_offset, handle)) > 0) { - response.encode(&buffer[0], buffer.size()); + if ((ret = this->fuse_read(path.c_str(), buffer.data(), read_size, + read_offset, handle)) > 0) { + response.encode(buffer.data(), buffer.size()); } return ret; }}); @@ -911,10 +909,10 @@ public: if (this->has_open_directory(client_id, reinterpret_cast(handle))) { - std::string filePath; - ret = this->fuse_readdir(&path[0], offset, handle, filePath); + std::string file_path; + ret = this->fuse_readdir(path.c_str(), offset, handle, file_path); if (ret == 0) { - response.encode(filePath); + response.encode(file_path); } } else { ret = -EBADF; @@ -935,7 +933,7 @@ public: remote::file_handle handle; DECODE_OR_RETURN(request, handle); - return this->fuse_release(&path[0], handle); + return this->fuse_release(path.c_str(), handle); }}); handler_lookup_.insert( {"::fuse_releasedir", @@ -950,7 +948,7 @@ public: remote::file_handle handle; DECODE_OR_RETURN(request, handle); - ret = this->fuse_releasedir(&path[0], handle); + ret = this->fuse_releasedir(path.c_str(), handle); if (this->remove_directory(client_id, reinterpret_cast(handle))) { return ret; @@ -968,7 +966,8 @@ public: std::string name; DECODE_OR_RETURN(request, name); - return this->fuse_removexattr(&path[0], &name[0]); + return this->fuse_removexattr(path.c_str(), + &name[0]); }});*/ handler_lookup_.insert( {"::fuse_rename", @@ -1093,7 +1092,7 @@ public: std::int32_t flags; DECODE_OR_RETURN(request, flags); - ret = this->fuse_setxattr(&path[0], &name[0], + ret = this->fuse_setxattr(path.c_str(), &name[0], &value[0], size, flags); } return ret; @@ -1125,8 +1124,8 @@ public: std::uint32_t position; DECODE_OR_RETURN(request, position); - ret = this->fuse_setxattr_osx(&path[0], &name[0], - &value[0], size, flags, position); + ret = this->fuse_setxattr_osx(path.c_str(), + &name[0], &value[0], size, flags, position); } return ret; }});*/ @@ -1430,12 +1429,6 @@ protected: path = utils::path::create_api_path(path.substr(mount_location_.size())); return path; } - - void delete_open_directory(void *dir) override { - if (dir != nullptr) { - delete reinterpret_cast(dir); - } - } }; } // namespace repertory diff --git a/repertory/librepertory/include/drives/winfsp/remotewinfsp/remote_client.hpp b/repertory/librepertory/include/drives/winfsp/remotewinfsp/remote_client.hpp index bedf27df..da3eed0e 100644 --- a/repertory/librepertory/include/drives/winfsp/remotewinfsp/remote_client.hpp +++ b/repertory/librepertory/include/drives/winfsp/remotewinfsp/remote_client.hpp @@ -46,9 +46,6 @@ private: static auto to_handle(PVOID file_desc) -> native_handle; #endif -protected: - void delete_open_directory(void * /*dir*/) override {} - public: auto json_create_directory_snapshot(const std::string &path, json &json_data) -> packet::error_type override; diff --git a/repertory/librepertory/src/drives/directory_cache.cpp b/repertory/librepertory/src/drives/directory_cache.cpp index f7f4e057..c475cd70 100644 --- a/repertory/librepertory/src/drives/directory_cache.cpp +++ b/repertory/librepertory/src/drives/directory_cache.cpp @@ -22,9 +22,8 @@ #include "drives/directory_cache.hpp" #include "drives/directory_iterator.hpp" -#include "events/event_system.hpp" -#include "events/events.hpp" #include "types/repertory.hpp" +#include "utils/utils.hpp" namespace repertory { void directory_cache::execute_action(const std::string &api_path, @@ -35,18 +34,17 @@ void directory_cache::execute_action(const std::string &api_path, } } -auto directory_cache::get_directory(directory_iterator *iterator) +auto directory_cache::get_directory(std::uint64_t handle) -> std::shared_ptr { - if (iterator != nullptr) { - recur_mutex_lock directory_lock(directory_mutex_); - const auto it = - std::find_if(directory_lookup_.begin(), directory_lookup_.end(), - [&iterator](const auto &kv) -> bool { - return kv.second.iterator.get() == iterator; - }); - if (it != directory_lookup_.end()) { - return it->second.iterator; - } + recur_mutex_lock directory_lock(directory_mutex_); + auto it = std::find_if(directory_lookup_.begin(), directory_lookup_.end(), + [handle](auto &&kv) -> bool { + auto &&handles = kv.second.handles; + return std::find(handles.begin(), handles.end(), + handle) != kv.second.handles.end(); + }); + if (it != directory_lookup_.end()) { + return it->second.iterator; } return nullptr; @@ -65,16 +63,18 @@ auto directory_cache::remove_directory(const std::string &api_path) return ret; } -void directory_cache::remove_directory(directory_iterator *iterator) { - if (iterator != nullptr) { - recur_mutex_lock directory_lock(directory_mutex_); - const auto it = - std::find_if(directory_lookup_.begin(), directory_lookup_.end(), - [&iterator](const auto &kv) -> bool { - return kv.second.iterator.get() == iterator; - }); - if (it != directory_lookup_.end()) { - directory_lookup_.erase(it->first); +void directory_cache::remove_directory(std::uint64_t handle) { + recur_mutex_lock directory_lock(directory_mutex_); + auto it = std::find_if(directory_lookup_.begin(), directory_lookup_.end(), + [handle](auto &&kv) -> bool { + auto &&handles = kv.second.handles; + return std::find(handles.begin(), handles.end(), + handle) != kv.second.handles.end(); + }); + if (it != directory_lookup_.end()) { + utils::remove_element_from(it->second.handles, handle); + if (it->second.handles.empty()) { + directory_lookup_.erase(it); } } } @@ -102,8 +102,11 @@ void directory_cache::service_function() { } void directory_cache::set_directory( - const std::string &api_path, std::shared_ptr iterator) { + const std::string &api_path, std::uint64_t handle, + std::shared_ptr iterator) { recur_mutex_lock directory_lock(directory_mutex_); - directory_lookup_[api_path] = {std::move(iterator)}; + auto &entry = directory_lookup_[api_path]; + entry.iterator = std::move(iterator); + entry.handles.push_back(handle); } } // namespace repertory diff --git a/repertory/librepertory/src/drives/fuse/fuse_drive.cpp b/repertory/librepertory/src/drives/fuse/fuse_drive.cpp index 34e20797..d41b42fc 100644 --- a/repertory/librepertory/src/drives/fuse/fuse_drive.cpp +++ b/repertory/librepertory/src/drives/fuse/fuse_drive.cpp @@ -669,8 +669,8 @@ auto fuse_drive::opendir_impl(std::string api_path, } auto iter = std::make_shared(std::move(list)); - file_info->fh = reinterpret_cast(iter.get()); - directory_cache_->set_directory(api_path, iter); + file_info->fh = fm_->get_next_handle(); + directory_cache_->set_directory(api_path, file_info->fh, iter); return api_error::success; } @@ -716,8 +716,7 @@ auto fuse_drive::readdir_impl(std::string api_path, void *buf, return res; } - auto iter = directory_cache_->get_directory( - reinterpret_cast(file_info->fh)); + auto iter = directory_cache_->get_directory(file_info->fh); if (iter == nullptr) { return api_error::invalid_handle; } @@ -751,13 +750,12 @@ auto fuse_drive::release_impl(std::string /*api_path*/, auto fuse_drive::releasedir_impl( std::string /*api_path*/, struct fuse_file_info *file_info) -> api_error { - auto iter = directory_cache_->get_directory( - reinterpret_cast(file_info->fh)); + auto iter = directory_cache_->get_directory(file_info->fh); if (iter == nullptr) { return api_error::invalid_handle; } - directory_cache_->remove_directory(iter.get()); + directory_cache_->remove_directory(file_info->fh); return api_error::success; } diff --git a/repertory/librepertory/src/drives/fuse/remotefuse/remote_server.cpp b/repertory/librepertory/src/drives/fuse/remotefuse/remote_server.cpp index e0ad1e0b..8a36c461 100644 --- a/repertory/librepertory/src/drives/fuse/remotefuse/remote_server.cpp +++ b/repertory/librepertory/src/drives/fuse/remotefuse/remote_server.cpp @@ -73,12 +73,6 @@ auto remote_server::construct_path(const std::wstring &path) -> std::string { return construct_path(utils::string::to_utf8(path)); } -void remote_server::delete_open_directory(void *dir) { - directory_cache_.remove_directory( - reinterpret_cast(dir)); - remote_server_base::delete_open_directory(dir); -} - auto remote_server::empty_as_zero(const json &data) -> std::string { if (data.empty() || data.get().empty()) { return "0"; @@ -86,6 +80,14 @@ auto remote_server::empty_as_zero(const json &data) -> std::string { return data.get(); } +auto remote_server::get_next_handle() -> std::uint64_t { + if (++next_handle_ == 0U) { + return ++next_handle_; + } + + return next_handle_; +} + auto remote_server::populate_file_info(const std::string &api_path, remote::file_info &file_info) -> packet::error_type { @@ -621,9 +623,9 @@ auto remote_server::fuse_opendir(const char *path, remote::file_handle &handle) auto list = drive_.get_directory_items(utils::path::create_api_path(path)); auto iter = std::make_shared(std::move(list)); - directory_cache_.set_directory(path, iter); + handle = get_next_handle(); + directory_cache_.set_directory(path, handle, iter); - handle = reinterpret_cast(iter.get()); res = 0; errno = 0; } @@ -682,8 +684,7 @@ auto remote_server::fuse_readdir(const char *path, errno = ERANGE; res = -1; } else { - auto iter = directory_cache_.get_directory( - reinterpret_cast(handle)); + auto iter = directory_cache_.get_directory(handle); if (iter != nullptr) { res = iter->get(static_cast(offset), item_path); } else { @@ -721,8 +722,7 @@ auto remote_server::fuse_releasedir( const auto file_path = construct_path(path); - directory_cache_.remove_directory( - reinterpret_cast(handle)); + directory_cache_.remove_directory(handle); RAISE_REMOTE_FUSE_SERVER_EVENT(function_name, file_path, 0); return 0; @@ -1569,9 +1569,10 @@ auto remote_server::json_create_directory_snapshot( if (utils::file::is_directory(file_path)) { auto list = drive_.get_directory_items(api_path); auto iter = std::make_shared(std::move(list)); - directory_cache_.set_directory(api_path, iter); + auto handle = get_next_handle(); + directory_cache_.set_directory(api_path, handle, iter); - json_data["handle"] = reinterpret_cast(iter.get()); + json_data["handle"] = handle; json_data["path"] = path; json_data["page_count"] = utils::divide_with_ceiling( iter->get_count(), REPERTORY_DIRECTORY_PAGE_SIZE); @@ -1589,22 +1590,25 @@ auto remote_server::json_read_directory_snapshot( std::uint32_t page, json &json_data) -> packet::error_type { constexpr const auto *function_name = static_cast(__FUNCTION__); + int res{-EBADF}; + const auto file_path = construct_path(path); - auto *iterator = reinterpret_cast(handle); - std::size_t offset{}; - int res{}; - json item_json; - while ((json_data["directory_list"].size() < REPERTORY_DIRECTORY_PAGE_SIZE) && - (res = iterator->get_json( - (page * REPERTORY_DIRECTORY_PAGE_SIZE) + offset++, item_json)) == - 0) { - json_data["directory_list"].emplace_back(item_json); + 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"] = reinterpret_cast(iterator); - json_data["path"] = path; - json_data["page"] = page; - json_data["page_count"] = utils::divide_with_ceiling( - iterator->get_count(), REPERTORY_DIRECTORY_PAGE_SIZE); auto ret = ((res < 0) ? -errno : 0); RAISE_REMOTE_FUSE_SERVER_EVENT(function_name, file_path, ret); return ret; @@ -1616,8 +1620,7 @@ auto remote_server::json_release_directory_snapshot( constexpr const auto *function_name = static_cast(__FUNCTION__); const auto file_path = construct_path(path); - directory_cache_.remove_directory( - reinterpret_cast(handle)); + directory_cache_.remove_directory(handle); RAISE_REMOTE_FUSE_SERVER_EVENT(function_name, file_path, 0); return 0; } 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 e55962e6..3e70795c 100644 --- a/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp +++ b/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp @@ -209,7 +209,6 @@ auto remote_open_file_table::remove_directory(const std::string &client_id, auto &list = directory_lookup_[client_id]; if (utils::collection_includes(list, dir)) { utils::remove_element_from(list, dir); - delete_open_directory(dir); if (directory_lookup_[client_id].empty()) { directory_lookup_.erase(client_id); }