diff --git a/Common/File/PathBrowser.cpp b/Common/File/PathBrowser.cpp index 008e0f5394..2928705514 100644 --- a/Common/File/PathBrowser.cpp +++ b/Common/File/PathBrowser.cpp @@ -123,6 +123,7 @@ PathBrowser::~PathBrowser() { } void PathBrowser::SetPath(const Path &path) { + std::lock_guard guard(pendingLock_); path_ = path; ApplyRestriction(); HandlePath(); @@ -130,18 +131,20 @@ void PathBrowser::SetPath(const Path &path) { void PathBrowser::RestrictToRoot(const Path &root) { VERBOSE_LOG(Log::IO, "Restricting to root: %s", root.c_str()); + std::lock_guard guard(pendingLock_); restrictedRoot_ = root; } void PathBrowser::HandlePath() { if (!path_.empty() && path_.ToString()[0] == '!') { - if (pendingActive_) - ResetPending(); + if (pendingActive_) { + pendingCancel_ = true; + pendingPath_.clear(); + } ready_ = true; return; } - std::lock_guard guard(pendingLock_); ready_ = false; pendingActive_ = true; pendingCancel_ = false; @@ -169,21 +172,24 @@ void PathBrowser::HandlePath() { } lastPath = pendingPath_; if (lastPath.Type() == PathType::HTTP) { + std::string userAgentCopy = userAgent_; guard.unlock(); results.clear(); - success_ = LoadRemoteFileList(lastPath, userAgent_, &pendingCancel_, results); + bool tempSuccess = LoadRemoteFileList(lastPath, userAgentCopy, &pendingCancel_, results); guard.lock(); + success_ = tempSuccess; } else if (lastPath.empty()) { results.clear(); success_ = true; } else { guard.unlock(); results.clear(); - success_ = File::GetFilesInDir(lastPath, &results, nullptr); + bool tempSuccess = File::GetFilesInDir(lastPath, &results, nullptr); + guard.lock(); + success_ = tempSuccess; if (!success_) { WARN_LOG(Log::IO, "PathBrowser: Failed to list directory: %s", lastPath.c_str()); } - guard.lock(); } if (pendingPath_ == lastPath) { @@ -198,15 +204,9 @@ void PathBrowser::HandlePath() { }); } -void PathBrowser::ResetPending() { - std::lock_guard guard(pendingLock_); - pendingCancel_ = true; - pendingPath_.clear(); -} - bool PathBrowser::GetListing(std::vector &fileInfo, const char *extensionFilter, bool *cancel) { std::unique_lock guard(pendingLock_); - while (!IsListingReady() && (!cancel || !*cancel)) { + while (!ready_ && (!cancel || !*cancel)) { // In case cancel changes, just sleep. TODO: Replace with condition variable. guard.unlock(); sleep_ms(50, "pathbrowser-poll"); @@ -224,7 +224,8 @@ void PathBrowser::ApplyRestriction() { } } -bool PathBrowser::CanNavigateUp() { +bool PathBrowser::CanNavigateUp() const { + std::unique_lock guard(pendingLock_); if (path_ == restrictedRoot_) { return false; } @@ -233,21 +234,26 @@ bool PathBrowser::CanNavigateUp() { void PathBrowser::NavigateUp() { _dbg_assert_(CanNavigateUp()); + + std::unique_lock guard(pendingLock_); path_ = path_.NavigateUp(); ApplyRestriction(); } // TODO: Support paths like "../../hello" void PathBrowser::Navigate(std::string_view path) { - if (path == ".") + std::unique_lock guard(pendingLock_); + if (path == ".") { + // Same directory, nothing to do. return; - if (path == "..") { - NavigateUp(); - } else { - if (path.size() >= 2 && path[1] == ':' && path_.IsRoot()) - path_ = Path(path); - else - path_ = path_ / path; } + if (path == "..") { + path_ = path_.NavigateUp(); + } else if (path.size() >= 2 && path[1] == ':' && path_.IsRoot()) { + path_ = Path(path); + } else { + path_ = path_ / path; + } + ApplyRestriction(); HandlePath(); } diff --git a/Common/File/PathBrowser.h b/Common/File/PathBrowser.h index 8372f25a70..0fdee76888 100644 --- a/Common/File/PathBrowser.h +++ b/Common/File/PathBrowser.h @@ -11,9 +11,9 @@ #include "Common/File/DirListing.h" #include "Common/File/Path.h" -// Abstraction above path that lets you navigate easily. +// Abstraction above path that lets you navigate easily and get listings off-thread. // "/" is a special path that means the root of the file system. On Windows, -// listing this will yield drives. +// listing "/" will yield drives. class PathBrowser { public: PathBrowser() {} @@ -24,11 +24,14 @@ public: HandlePath(); } bool IsListingReady() const { + std::lock_guard guard(pendingLock_); return ready_; } + + // If called before IsListingReady() returns true, will block (becomes synchronous). Don't do that. bool GetListing(std::vector &fileInfo, const char *filter = nullptr, bool *cancel = nullptr); - bool CanNavigateUp(); + bool CanNavigateUp() const; void NavigateUp(); void Navigate(std::string_view subdir); @@ -38,6 +41,7 @@ public: } void SetUserAgent(std::string_view s) { + std::lock_guard guard(pendingLock_); userAgent_ = s; } void RestrictToRoot(const Path &root); @@ -45,12 +49,12 @@ public: return path_.empty(); } bool Success() const { + std::lock_guard guard(pendingLock_); return success_; } private: void HandlePath(); - void ResetPending(); void ApplyRestriction(); Path path_; @@ -59,7 +63,7 @@ private: std::string userAgent_; std::vector pendingFiles_; std::condition_variable pendingCond_; - std::mutex pendingLock_; + mutable std::mutex pendingLock_; std::thread pendingThread_; bool pendingActive_ = false; bool pendingCancel_ = false; @@ -67,4 +71,3 @@ private: bool ready_ = false; bool success_ = true; }; - diff --git a/Common/System/System.h b/Common/System/System.h index 38d9c2a0e3..57c118dc8c 100644 --- a/Common/System/System.h +++ b/Common/System/System.h @@ -49,7 +49,6 @@ enum class LaunchUrlType { EMAIL_ADDRESS, LOCAL_FILE, LOCAL_FOLDER, // Shows the folder. Not supported on all systems of course. - AUTO, }; void System_Vibrate(int length_ms);