Remove race in idle unmount

encfs/Context.cpp

@@ -34,6 +34,8 @@ EncFS_Context::EncFS_Context() {
   pthread_mutex_init(&contextMutex, nullptr);
 
   usageCount = 0;
+  idleCount = -1;
+  isUnmounting = false;
   currentFuseFh = 1;
 }
 
@@ -46,10 +48,14 @@ EncFS_Context::~EncFS_Context() {
   openFiles.clear();
 }
 std::shared_ptr<DirNode> EncFS_Context::getRoot(int *errCode) {
-  std::shared_ptr<DirNode> ret;
+  std::shared_ptr<DirNode> ret = nullptr;
   do {
     {
       Lock lock(contextMutex);
+      if (isUnmounting) {
+        *errCode = -EBUSY;
+        break;      	
+      }
       ret = root;
       ++usageCount;
     }
@@ -75,15 +81,44 @@ void EncFS_Context::setRoot(const std::shared_ptr<DirNode> &r) {
   }
 }
 
-bool EncFS_Context::isMounted() { return root != nullptr; }
-
-void EncFS_Context::getAndResetUsageCounter(int *usage, int *openCount) {
+// This function is called periodically by the idle monitoring thread.
+// It checks for inactivity and unmount the FS after enough inactive cycles have passed.
+// Returns true if FS has really been unmounted, false otherwise.
+bool EncFS_Context::usageAndUnmount(int timeoutCycles) {
   Lock lock(contextMutex);
 
-  *usage = usageCount;
-  usageCount = 0;
+  if (root != nullptr) {
 
-  *openCount = openFiles.size();
+    if (usageCount == 0) {
+      ++idleCount;
+    }
+    else {
+      idleCount = 0;
+    }
+    VLOG(1) << "idle cycle count: " << idleCount << ", timeout at "
+            << timeoutCycles;
+
+    usageCount = 0;
+
+    if (idleCount < timeoutCycles) {
+      return false;
+    }
+
+    if (!openFiles.empty()) {
+      if (idleCount % timeoutCycles == 0) {
+        RLOG(WARNING) << "Filesystem inactive, but " << openFiles.size()
+                      << " files opened: " << this->opts->mountPoint;
+      }
+      return false;
+    }
+    if (!this->opts->mountOnDemand) {
+      isUnmounting = true;
+    }
+    lock.~Lock();
+    return unmountFS(this);
+
+  }
+  return false;
 }
 
 std::shared_ptr<FileNode> EncFS_Context::lookupNode(const char *path) {

encfs/Context.h

@@ -46,7 +46,7 @@ class EncFS_Context {
 
   std::shared_ptr<FileNode> lookupNode(const char *path);
 
-  void getAndResetUsageCounter(int *usage, int *openCount);
+  bool usageAndUnmount(int timeoutCycles);
 
   void putNode(const char *path, const std::shared_ptr<FileNode> &node);
 
@@ -56,7 +56,6 @@ class EncFS_Context {
 
   void setRoot(const std::shared_ptr<DirNode> &root);
   std::shared_ptr<DirNode> getRoot(int *err);
-  bool isMounted();
 
   std::shared_ptr<EncFS_Args> args;
   std::shared_ptr<EncFS_Opts> opts;
@@ -92,6 +91,8 @@ class EncFS_Context {
   FileMap openFiles;
 
   int usageCount;
+  int idleCount;
+  bool isUnmounting;
   std::shared_ptr<DirNode> root;
 
   std::atomic<std::uint64_t> currentFuseFh;
@@ -99,6 +100,7 @@ class EncFS_Context {
 };
 
 int remountFS(EncFS_Context *ctx);
+bool unmountFS(EncFS_Context *ctx);
 
 }  // namespace encfs
 

encfs/FileUtils.cpp

@@ -1681,4 +1681,23 @@ int remountFS(EncFS_Context *ctx) {
   return -EACCES;
 }
 
+bool unmountFS(EncFS_Context *ctx) {
+  if (ctx->opts->mountOnDemand) {
+    VLOG(1) << "Detaching filesystem due to inactivity: "
+            << ctx->opts->mountPoint;
+
+    ctx->setRoot(std::shared_ptr<DirNode>());
+    return false;
+  }
+// Time to unmount!
+#if FUSE_USE_VERSION < 30
+  fuse_unmount(ctx->opts->mountPoint.c_str(), nullptr);
+#else
+  fuse_unmount(fuse_get_context()->fuse);
+#endif
+  // fuse_unmount succeeds and returns void
+  RLOG(INFO) << "Filesystem inactive, unmounted: " << ctx->opts->mountPoint;
+  return true;
+}
+
 }  // namespace encfs

encfs/main.cpp

@@ -744,14 +744,12 @@ int main(int argc, char *argv[]) {
     having the filesystem unmounted from underneath open files!
 */
 const int ActivityCheckInterval = 10;
-static bool unmountFS(EncFS_Context *ctx);
 
 static void *idleMonitor(void *_arg) {
   auto *ctx = (EncFS_Context *)_arg;
   std::shared_ptr<EncFS_Args> arg = ctx->args;
 
   const int timeoutCycles = 60 * arg->idleTimeout / ActivityCheckInterval;
-  int idleCycles = -1;
 
   bool unmountres = false;
 
@@ -762,36 +760,12 @@ static void *idleMonitor(void *_arg) {
   pthread_mutex_lock(&ctx->wakeupMutex);
 
   while (ctx->running) {
-    int usage, openCount;
-    ctx->getAndResetUsageCounter(&usage, &openCount);
-
-    if (usage == 0 && ctx->isMounted()) {
-      ++idleCycles;
-    } else {
-      if (idleCycles >= timeoutCycles) {
-        RLOG(INFO) << "Filesystem no longer inactive: "
-                   << arg->opts->mountPoint;
-      }
-      idleCycles = 0;
+    unmountres = ctx->usageAndUnmount(timeoutCycles);
+    if (unmountres) {
+      pthread_cond_wait(&ctx->wakeupCond, &ctx->wakeupMutex);
+      break;
     }
 
-    if (idleCycles >= timeoutCycles) {
-      if (openCount == 0) {
-        unmountres = unmountFS(ctx);
-        if (unmountres) {
-          // wait for main thread to wake us up
-          pthread_cond_wait(&ctx->wakeupCond, &ctx->wakeupMutex);
-          break;
-        }
-      } else {
-        RLOG(WARNING) << "Filesystem inactive, but " << openCount
-                      << " files opened: " << arg->opts->mountPoint;
-      }
-    }
-
-    VLOG(1) << "idle cycle count: " << idleCycles << ", timeout after "
-            << timeoutCycles;
-
     struct timeval currentTime;
     gettimeofday(&currentTime, nullptr);
     struct timespec wakeupTime;
@@ -802,8 +776,8 @@ static void *idleMonitor(void *_arg) {
 
   pthread_mutex_unlock(&ctx->wakeupMutex);
 
-  // If we are here FS has been unmounted, so if we did not unmount ourselves
-  // (manual, kill...), notify
+  // If we are here FS has been unmounted, so if the idleMonitor did not unmount itself,
+  // let's notify (certainly due to a kill signal, a manual unmount...)
   if (!unmountres) {
     RLOG(INFO) << "Filesystem unmounted: " << arg->opts->mountPoint;
   }
@@ -812,23 +786,3 @@ static void *idleMonitor(void *_arg) {
 
   return nullptr;
 }
-
-static bool unmountFS(EncFS_Context *ctx) {
-  std::shared_ptr<EncFS_Args> arg = ctx->args;
-  if (arg->opts->mountOnDemand) {
-    VLOG(1) << "Detaching filesystem due to inactivity: "
-            << arg->opts->mountPoint;
-
-    ctx->setRoot(std::shared_ptr<DirNode>());
-    return false;
-  }
-// Time to unmount!
-#if FUSE_USE_VERSION < 30
-  fuse_unmount(arg->opts->mountPoint.c_str(), nullptr);
-#else
-  fuse_unmount(fuse_get_context()->fuse);
-#endif
-  // fuse_unmount succeeds and returns void
-  RLOG(INFO) << "Filesystem inactive, unmounted: " << arg->opts->mountPoint;
-  return true;
-}