From 1c273ecba57d95c529f21d72c7615e2bb8a911cf Mon Sep 17 00:00:00 2001 From: Ben RUBSON Date: Wed, 15 Nov 2017 21:28:37 +0100 Subject: [PATCH] Remove race in idle unmount --- encfs/Context.cpp | 49 ++++++++++++++++++++++++++++++++------ encfs/Context.h | 6 +++-- encfs/FileUtils.cpp | 19 +++++++++++++++ encfs/main.cpp | 58 +++++---------------------------------------- 4 files changed, 71 insertions(+), 61 deletions(-) diff --git a/encfs/Context.cpp b/encfs/Context.cpp index 781661d..60606bd 100644 --- a/encfs/Context.cpp +++ b/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 EncFS_Context::getRoot(int *errCode) { - std::shared_ptr ret; + std::shared_ptr 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 &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 EncFS_Context::lookupNode(const char *path) { diff --git a/encfs/Context.h b/encfs/Context.h index d77334f..3d84d37 100644 --- a/encfs/Context.h +++ b/encfs/Context.h @@ -46,7 +46,7 @@ class EncFS_Context { std::shared_ptr lookupNode(const char *path); - void getAndResetUsageCounter(int *usage, int *openCount); + bool usageAndUnmount(int timeoutCycles); void putNode(const char *path, const std::shared_ptr &node); @@ -56,7 +56,6 @@ class EncFS_Context { void setRoot(const std::shared_ptr &root); std::shared_ptr getRoot(int *err); - bool isMounted(); std::shared_ptr args; std::shared_ptr opts; @@ -92,6 +91,8 @@ class EncFS_Context { FileMap openFiles; int usageCount; + int idleCount; + bool isUnmounting; std::shared_ptr root; std::atomic currentFuseFh; @@ -99,6 +100,7 @@ class EncFS_Context { }; int remountFS(EncFS_Context *ctx); +bool unmountFS(EncFS_Context *ctx); } // namespace encfs diff --git a/encfs/FileUtils.cpp b/encfs/FileUtils.cpp index 35bc65b..bf14f03 100644 --- a/encfs/FileUtils.cpp +++ b/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()); + 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 diff --git a/encfs/main.cpp b/encfs/main.cpp index 37ec751..bec900e 100644 --- a/encfs/main.cpp +++ b/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 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(¤tTime, 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 arg = ctx->args; - if (arg->opts->mountOnDemand) { - VLOG(1) << "Detaching filesystem due to inactivity: " - << arg->opts->mountPoint; - - ctx->setRoot(std::shared_ptr()); - 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; -}