Fixed a deadlock in the FdManager::ChangeEntityToTempPath method call

src/fdcache.cpp

@@ -78,6 +78,7 @@ FdManager       FdManager::singleton;
 std::mutex      FdManager::fd_manager_lock;
 std::mutex      FdManager::cache_cleanup_lock;
 std::mutex      FdManager::reserved_diskspace_lock;
+std::mutex      FdManager::except_entmap_lock;
 std::string     FdManager::cache_dir;
 bool            FdManager::check_cache_dir_exist(false);
 off_t           FdManager::free_disk_space = 0;
@@ -465,6 +466,7 @@ FdManager::~FdManager()
             S3FS_PRN_WARN("To exit with the cache file opened: path=%s, refcnt=%d", ent->GetPath().c_str(), ent->GetOpenCount());
         }
         fent.clear();
+        except_fent.clear();
     }else{
         abort();
     }
@@ -478,6 +480,8 @@ FdEntity* FdManager::GetFdEntityHasLock(const char* path, int& existfd, bool new
         return nullptr;
     }
 
+    UpdateEntityToTempPath();
+
     fdent_map_t::iterator iter = fent.find(path);
     if(fent.end() != iter && iter->second){
         if(-1 == existfd){
@@ -534,6 +538,8 @@ FdEntity* FdManager::Open(int& fd, const char* path, const headers_t* pmeta, off
 
     const std::lock_guard<std::mutex> lock(FdManager::fd_manager_lock);
 
+    UpdateEntityToTempPath();
+
     // search in mapping by key(path)
     fdent_map_t::iterator iter = fent.find(path);
     if(fent.end() == iter && !force_tmpfile && !FdManager::IsCacheDir()){
@@ -620,6 +626,8 @@ FdEntity* FdManager::GetExistFdEntity(const char* path, int existfd)
 
     const std::lock_guard<std::mutex> lock(FdManager::fd_manager_lock);
 
+    UpdateEntityToTempPath();
+
     // search from all entity.
     for(fdent_map_t::iterator iter = fent.begin(); iter != fent.end(); ++iter){
         if(iter->second && iter->second->FindPseudoFd(existfd)){
@@ -656,6 +664,8 @@ int FdManager::GetPseudoFdCount(const char* path)
         return 0;
     }
 
+    UpdateEntityToTempPath();
+
     // search from all entity.
     for(fdent_map_t::iterator iter = fent.begin(); iter != fent.end(); ++iter){
         if(iter->second && iter->second->GetPath() == path){
@@ -671,6 +681,8 @@ void FdManager::Rename(const std::string &from, const std::string &to)
 {
     const std::lock_guard<std::mutex> lock(FdManager::fd_manager_lock);
 
+    UpdateEntityToTempPath();
+
     fdent_map_t::iterator iter = fent.find(from);
     if(fent.end() == iter && !FdManager::IsCacheDir()){
         // If the cache directory is not specified, s3fs opens a temporary file
@@ -715,6 +727,8 @@ bool FdManager::Close(FdEntity* ent, int fd)
     }
     const std::lock_guard<std::mutex> lock(FdManager::fd_manager_lock);
 
+    UpdateEntityToTempPath();
+
     for(fdent_map_t::iterator iter = fent.begin(); iter != fent.end(); ++iter){
         if(iter->second.get() == ent){
             ent->Close(fd);
@@ -737,23 +751,46 @@ bool FdManager::Close(FdEntity* ent, int fd)
     return false;
 }
 
-bool FdManager::ChangeEntityToTempPath(const FdEntity* ent, const char* path)
+bool FdManager::ChangeEntityToTempPath(FdEntity* ent, const char* path)
 {
-    const std::lock_guard<std::mutex> lock(FdManager::fd_manager_lock);
+    const std::lock_guard<std::mutex> lock(FdManager::except_entmap_lock);
+    except_fent[path] = ent;
+    return true;
+}
 
-    for(fdent_map_t::iterator iter = fent.begin(); iter != fent.end(); ){
+// [NOTE]
-        if(iter->second.get() == ent){
+// FdManager::fd_manager_lock should be locked by the caller.
+//
+bool FdManager::UpdateEntityToTempPath()
+{
+    const std::lock_guard<std::mutex> lock(FdManager::except_entmap_lock);
+
+    for(fdent_direct_map_t::iterator except_iter = except_fent.begin(); except_iter != except_fent.end(); ){
         std::string tmppath;
-            FdManager::MakeRandomTempPath(path, tmppath);
+        FdManager::MakeRandomTempPath(except_iter->first.c_str(), tmppath);
+
+        fdent_map_t::iterator iter = fent.find(except_iter->first);
+        if(fent.end() != iter && iter->second.get() == except_iter->second){
             // Move the entry to the new key
             fent[tmppath] = std::move(iter->second);
             iter          = fent.erase(iter);
-            return true;
+            except_iter   = except_fent.erase(except_iter);
         }else{
-            ++iter;
+            // [NOTE]
+            // ChangeEntityToTempPath method is called and the FdEntity pointer
+            // set into except_fent is mapped into fent.
+            // And since this method is always called before manipulating fent,
+            // it will not enter here.
+            // Thus, if it enters here, a warning is output.
+            //
+            S3FS_PRN_WARN("For some reason the FdEntity pointer(for %s) is not found in the fent map. Recovery procedures are being performed, but the cause needs to be identified.", except_iter->first.c_str());
+
+            // Add the entry for recovery procedures
+            fent[tmppath] = std::unique_ptr<FdEntity>(except_iter->second);
+            except_iter   = except_fent.erase(except_iter);
         }
     }
-    return false;
+    return true;
 }
 
 void FdManager::CleanupCacheDir()
@@ -807,6 +844,8 @@ void FdManager::CleanupCacheDirInternal(const std::string &path)
                 S3FS_PRN_INFO("could not get fd_manager_lock when clean up file(%s), then skip it.", next_path.c_str());
                 continue;
             }
+            UpdateEntityToTempPath();
+
             fdent_map_t::iterator iter = fent.find(next_path);
             if(fent.end() == iter) {
                 S3FS_PRN_DBG("cleaned up: %s", next_path.c_str());
@@ -917,6 +956,8 @@ bool FdManager::RawCheckAllCache(FILE* fp, const char* cache_stat_top_dir, const
             {
                 const std::lock_guard<std::mutex> lock(FdManager::fd_manager_lock);
 
+                UpdateEntityToTempPath();
+
                 fdent_map_t::iterator iter = fent.find(object_file_path);
                 if(fent.end() != iter){
                     // This file is opened now, then we need to put warning message.

src/fdcache.h

@@ -36,6 +36,7 @@ class FdManager
       static std::mutex      fd_manager_lock;
       static std::mutex      cache_cleanup_lock;
       static std::mutex      reserved_diskspace_lock;
+      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
@@ -46,6 +47,7 @@ class FdManager
       static std::string     tmp_dir;
 
       fdent_map_t            fent;
+      fdent_direct_map_t     except_fent;           // A map of delayed deletion fdentity
 
   private:
       static off_t GetFreeDiskSpace(const char* path);
@@ -54,6 +56,7 @@ class FdManager
       static int GetVfsStat(const char* path, struct statvfs* vfsbuf);
 
       int GetPseudoFdCount(const char* path);
+      bool UpdateEntityToTempPath();
       void CleanupCacheDirInternal(const std::string &path = "");
       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);
 
@@ -106,7 +109,7 @@ class FdManager
       FdEntity* OpenExistFdEntity(const char* path, int& fd, int flags = O_RDONLY);
       void Rename(const std::string &from, const std::string &to);
       bool Close(FdEntity* ent, int fd);
-      bool ChangeEntityToTempPath(const FdEntity* ent, const char* path);
+      bool ChangeEntityToTempPath(FdEntity* ent, const char* path);
       void CleanupCacheDir();
 
       bool CheckAllCache();

src/fdcache_entity.h

@@ -219,7 +219,8 @@ class FdEntity
         bool ReplaceLastUpdateUntreatedPart(off_t front_start, off_t front_size, off_t behind_start, off_t behind_size);
 };
 
-typedef std::map<std::string, std::unique_ptr<FdEntity>> fdent_map_t;   // key=path, value=FdEntity*
+typedef std::map<std::string, std::unique_ptr<FdEntity>> fdent_map_t;           // key=path, value=unique_ptr<FdEntity>
+typedef std::map<std::string, FdEntity*>                 fdent_direct_map_t;    // key=path, value=FdEntity*
 
 #endif // S3FS_FDCACHE_ENTITY_H_