diff --git a/src/cache.cpp b/src/cache.cpp index ed448cb..0d211ef 100644 --- a/src/cache.cpp +++ b/src/cache.cpp @@ -706,9 +706,6 @@ bool StatCache::DelSymlinkHasLock(const std::string& key) return true; } -// [NOTE] -// Need to lock StatCache::stat_cache_lock before calling this method. -// bool StatCache::AddNotruncateCache(const std::string& key) { if(key.empty() || '/' == *key.rbegin()){ @@ -739,9 +736,6 @@ bool StatCache::AddNotruncateCache(const std::string& key) return true; } -// [NOTE] -// Need to lock StatCache::stat_cache_lock before calling this method. -// bool StatCache::DelNotruncateCache(const std::string& key) { if(key.empty() || '/' == *key.rbegin()){ diff --git a/src/cache.h b/src/cache.h index 3cd757c..614fc68 100644 --- a/src/cache.h +++ b/src/cache.h @@ -83,14 +83,14 @@ class StatCache private: static StatCache singleton; static std::mutex stat_cache_lock; - stat_cache_t stat_cache; + stat_cache_t stat_cache GUARDED_BY(stat_cache_lock); bool IsExpireTime; bool IsExpireIntervalType; // if this flag is true, cache data is updated at last access time. time_t ExpireTime; unsigned long CacheSize; bool IsCacheNoObject; - symlink_cache_t symlink_cache; - notruncate_dir_map_t notruncate_file_cache; + symlink_cache_t symlink_cache GUARDED_BY(stat_cache_lock); + notruncate_dir_map_t notruncate_file_cache GUARDED_BY(stat_cache_lock); StatCache(); ~StatCache(); @@ -102,8 +102,8 @@ class StatCache // Truncate symbolic link cache bool TruncateSymlink() REQUIRES(StatCache::stat_cache_lock); - bool AddNotruncateCache(const std::string& key); - bool DelNotruncateCache(const std::string& key); + bool AddNotruncateCache(const std::string& key) REQUIRES(stat_cache_lock); + bool DelNotruncateCache(const std::string& key) REQUIRES(stat_cache_lock); public: StatCache(const StatCache&) = delete; @@ -191,7 +191,7 @@ class StatCache const std::lock_guard lock(StatCache::stat_cache_lock); return DelSymlinkHasLock(key); } - bool DelSymlinkHasLock(const std::string& key); + bool DelSymlinkHasLock(const std::string& key) REQUIRES(stat_cache_lock); // Cache for Notruncate file bool GetNotruncateCache(const std::string& parentdir, notruncate_filelist_t& list); diff --git a/src/common.h b/src/common.h index 80e28dd..2843005 100644 --- a/src/common.h +++ b/src/common.h @@ -63,6 +63,9 @@ extern std::string instance_name; #define GUARDED_BY(x) \ THREAD_ANNOTATION_ATTRIBUTE(guarded_by(x)) +#define PT_GUARDED_BY(x) \ + THREAD_ANNOTATION_ATTRIBUTE(pt_guarded_by(x)) + #define REQUIRES(...) \ THREAD_ANNOTATION_ATTRIBUTE(requires_capability(__VA_ARGS__)) diff --git a/src/curl.h b/src/curl.h index df59b71..712b3eb 100644 --- a/src/curl.h +++ b/src/curl.h @@ -175,7 +175,7 @@ class S3fsCurl static long ipresolve_type; // this value is a libcurl symbol. // variables - CurlUniquePtr hCurl = {nullptr, curl_easy_cleanup}; + CurlUniquePtr hCurl PT_GUARDED_BY(curl_handles_lock) = {nullptr, curl_easy_cleanup}; REQTYPE type; // type of request std::string path; // target object path std::string base_path; // base path (for multi curl head request) @@ -205,7 +205,7 @@ class S3fsCurl std::string query_string; // request query string Semaphore *sem; std::mutex *completed_tids_lock; - std::vector *completed_tids; + std::vector *completed_tids PT_GUARDED_BY(*completed_tids_lock); s3fscurl_lazy_setup fpLazySetup; // curl options for lazy setting function CURLcode curlCode; // handle curl return diff --git a/src/curl_handlerpool.cpp b/src/curl_handlerpool.cpp index b7cd96f..3bd152c 100644 --- a/src/curl_handlerpool.cpp +++ b/src/curl_handlerpool.cpp @@ -35,6 +35,7 @@ bool CurlHandlerPool::Init() Destroy(); return false; } + const std::lock_guard lock(mLock); mPool.push_back(std::move(hCurl)); } return true; diff --git a/src/curl_handlerpool.h b/src/curl_handlerpool.h index 8476341..ac12447 100644 --- a/src/curl_handlerpool.h +++ b/src/curl_handlerpool.h @@ -27,6 +27,8 @@ #include #include +#include "common.h" + //---------------------------------------------- // Typedefs //---------------------------------------------- @@ -57,7 +59,7 @@ class CurlHandlerPool private: int mMaxHandlers; std::mutex mLock; - std::list mPool; + std::list mPool GUARDED_BY(mLock); }; #endif // S3FS_CURL_HANDLERPOOL_H_ diff --git a/src/curl_multi.h b/src/curl_multi.h index d62f6a7..69d4745 100644 --- a/src/curl_multi.h +++ b/src/curl_multi.h @@ -27,6 +27,8 @@ #include #include +#include "common.h" + //---------------------------------------------- // Typedef //---------------------------------------------- @@ -56,7 +58,7 @@ class S3fsMultiCurl void* pNotFoundCallbackParam; std::mutex completed_tids_lock; - std::vector completed_tids; + std::vector completed_tids GUARDED_BY(completed_tids_lock); private: bool ClearEx(bool is_all); diff --git a/src/fdcache.cpp b/src/fdcache.cpp index 94236d8..d2a83aa 100644 --- a/src/fdcache.cpp +++ b/src/fdcache.cpp @@ -651,10 +651,6 @@ FdEntity* FdManager::OpenExistFdEntity(const char* path, int& fd, int flags) return ent; } -// [NOTE] -// Returns the number of open pseudo fd. -// This method is called from GetOpenFdCount method which is already locked. -// int FdManager::GetPseudoFdCount(const char* path) { S3FS_PRN_DBG("[path=%s]", SAFESTRPTR(path)); @@ -757,9 +753,6 @@ bool FdManager::ChangeEntityToTempPath(FdEntity* ent, const char* path) return true; } -// [NOTE] -// FdManager::fd_manager_lock should be locked by the caller. -// bool FdManager::UpdateEntityToTempPath() { const std::lock_guard lock(FdManager::except_entmap_lock); diff --git a/src/fdcache.h b/src/fdcache.h index 0c8fed7..df20957 100644 --- a/src/fdcache.h +++ b/src/fdcache.h @@ -39,15 +39,15 @@ class FdManager static std::mutex except_entmap_lock; static std::string cache_dir; static bool check_cache_dir_exist; - static off_t free_disk_space; // limit free disk space - static off_t fake_used_disk_space; // difference between fake free disk space and actual at startup(for test/debug) + static off_t free_disk_space GUARDED_BY(reserved_diskspace_lock); // limit free disk space + static off_t fake_used_disk_space GUARDED_BY(reserved_diskspace_lock); // difference between fake free disk space and actual at startup(for test/debug) static std::string check_cache_output; static bool checked_lseek; static bool have_lseek_hole; static std::string tmp_dir; - fdent_map_t fent; - fdent_direct_map_t except_fent; // A map of delayed deletion fdentity + fdent_map_t fent GUARDED_BY(fd_manager_lock); + fdent_direct_map_t except_fent GUARDED_BY(except_entmap_lock); // A map of delayed deletion fdentity private: static off_t GetFreeDiskSpaceHasLock(const char* path) REQUIRES(FdManager::reserved_diskspace_lock); @@ -56,9 +56,10 @@ class FdManager static int GetVfsStat(const char* path, struct statvfs* vfsbuf); static off_t GetEnsureFreeDiskSpaceHasLock() REQUIRES(FdManager::reserved_diskspace_lock); - int GetPseudoFdCount(const char* path); - bool UpdateEntityToTempPath(); - void CleanupCacheDirInternal(const std::string &path = ""); + // Returns the number of open pseudo fd. + int GetPseudoFdCount(const char* path) REQUIRES(fd_manager_lock); + bool UpdateEntityToTempPath() REQUIRES(fd_manager_lock); + void CleanupCacheDirInternal(const std::string &path = "") REQUIRES(cache_cleanup_lock); bool RawCheckAllCache(FILE* fp, const char* cache_stat_top_dir, const char* sub_path, int& total_file_cnt, int& err_file_cnt, int& err_dir_cnt); public: diff --git a/src/fdcache_pseudofd.h b/src/fdcache_pseudofd.h index 58aaf91..6267f2b 100644 --- a/src/fdcache_pseudofd.h +++ b/src/fdcache_pseudofd.h @@ -24,6 +24,8 @@ #include #include +#include "common.h" + //------------------------------------------------ // Typdefs //------------------------------------------------ @@ -37,7 +39,7 @@ typedef std::vector pseudofd_list_t; class PseudoFdManager { private: - pseudofd_list_t pseudofd_list; + pseudofd_list_t pseudofd_list GUARDED_BY(pseudofd_list_lock); std::mutex pseudofd_list_lock; // protects pseudofd_list static PseudoFdManager& GetManager(); @@ -45,7 +47,7 @@ class PseudoFdManager PseudoFdManager() = default; ~PseudoFdManager() = default; - int GetUnusedMinPseudoFd() const; + int GetUnusedMinPseudoFd() const REQUIRES(pseudofd_list_lock); int CreatePseudoFd(); bool ReleasePseudoFd(int fd); diff --git a/src/fdcache_untreated.h b/src/fdcache_untreated.h index 73586dc..765c3aa 100644 --- a/src/fdcache_untreated.h +++ b/src/fdcache_untreated.h @@ -34,8 +34,8 @@ class UntreatedParts private: mutable std::mutex untreated_list_lock; // protects untreated_list - untreated_list_t untreated_list; - long last_tag = 0; // [NOTE] Use this to identify the latest updated part. + untreated_list_t untreated_list GUARDED_BY(untreated_list_lock); + long last_tag GUARDED_BY(untreated_list_lock) = 0; // [NOTE] Use this to identify the latest updated part. private: bool RowGetPart(off_t& start, off_t& size, off_t max_size, off_t min_size, bool lastpart) const; diff --git a/src/s3fs_cred.h b/src/s3fs_cred.h index 790cb45..88eb826 100644 --- a/src/s3fs_cred.h +++ b/src/s3fs_cred.h @@ -65,22 +65,22 @@ class S3fsCred bool load_iamrole; - std::string AWSAccessKeyId; // Protect exclusively - std::string AWSSecretAccessKey; // Protect exclusively - std::string AWSAccessToken; // Protect exclusively - time_t AWSAccessTokenExpire; // Protect exclusively + std::string AWSAccessKeyId GUARDED_BY(token_lock); + std::string AWSSecretAccessKey GUARDED_BY(token_lock); + std::string AWSAccessToken GUARDED_BY(token_lock); + time_t AWSAccessTokenExpire GUARDED_BY(token_lock); bool is_ecs; bool is_use_session_token; bool is_ibm_iam_auth; std::string IAM_cred_url; - int IAM_api_version; // Protect exclusively - std::string IAMv2_api_token; // Protect exclusively + int IAM_api_version GUARDED_BY(token_lock); + std::string IAMv2_api_token GUARDED_BY(token_lock); size_t IAM_field_count; std::string IAM_token_field; std::string IAM_expiry_field; - std::string IAM_role; // Protect exclusively + std::string IAM_role GUARDED_BY(token_lock); bool set_builtin_cred_opts; // true if options other than "credlib" is set std::string credlib; // credlib(name or path) diff --git a/src/threadpoolman.cpp b/src/threadpoolman.cpp index 4059084..27529c9 100644 --- a/src/threadpoolman.cpp +++ b/src/threadpoolman.cpp @@ -147,6 +147,8 @@ void ThreadPoolMan::SetExitFlag(bool exit_flag) bool ThreadPoolMan::StopThreads() { + const std::lock_guard lock(thread_list_lock); + if(thread_list.empty()){ S3FS_PRN_INFO("Any threads are running now, then nothing to do."); return true; @@ -195,6 +197,8 @@ bool ThreadPoolMan::StartThreads(int count) std::promise promise; std::future future = promise.get_future(); std::thread thread(ThreadPoolMan::Worker, this, std::move(promise)); + + const std::lock_guard lock(thread_list_lock); thread_list.emplace_back(std::move(thread), std::move(future)); } return true; diff --git a/src/threadpoolman.h b/src/threadpoolman.h index 3c5d906..db072a2 100644 --- a/src/threadpoolman.h +++ b/src/threadpoolman.h @@ -27,6 +27,7 @@ #include #include +#include "common.h" #include "psemaphore.h" //------------------------------------------------ @@ -66,9 +67,8 @@ class ThreadPoolMan Semaphore thpoolman_sem; std::mutex thread_list_lock; - std::vector>> thread_list; - - thpoolman_params_t instruction_list; + std::vector>> thread_list GUARDED_BY(thread_list_lock); + thpoolman_params_t instruction_list GUARDED_BY(thread_list_lock); private: static void Worker(ThreadPoolMan* psingleton, std::promise promise);