Fix #71: driver hangs when unmounting

The tidy thread that gets spawned when constructing the block cache
doesn't survive the fork to background when the driver is not being
run in foreground mode. This is a familar problem that we already
ran into with the cache worker threads. The fix is simple: just delay
the thread spawning until after we've forked to background.

include/dwarfs/block_cache.h

@@ -33,6 +33,7 @@
 namespace dwarfs {
 
 struct block_cache_options;
+struct cache_tidy_config;
 
 class block_range;
 class fs_section;
@@ -52,6 +53,10 @@ class block_cache {
 
   void set_num_workers(size_t num) { impl_->set_num_workers(num); }
 
+  void set_tidy_config(cache_tidy_config const& cfg) {
+    impl_->set_tidy_config(cfg);
+  }
+
   std::future<block_range>
   get(size_t block_no, size_t offset, size_t size) const {
     return impl_->get(block_no, offset, size);
@@ -65,6 +70,7 @@ class block_cache {
     virtual void insert(fs_section const& section) = 0;
     virtual void set_block_size(size_t size) = 0;
     virtual void set_num_workers(size_t num) = 0;
+    virtual void set_tidy_config(cache_tidy_config const& cfg) = 0;
     virtual std::future<block_range>
     get(size_t block_no, size_t offset, size_t length) const = 0;
   };

include/dwarfs/filesystem_v2.h

@@ -44,6 +44,7 @@ struct statvfs;
 
 namespace dwarfs {
 
+struct cache_tidy_config;
 struct filesystem_options;
 struct rewrite_options;
 struct iovec_read_buf;
@@ -152,6 +153,9 @@ class filesystem_v2 {
   std::optional<folly::ByteRange> header() const { return impl_->header(); }
 
   void set_num_workers(size_t num) { return impl_->set_num_workers(num); }
+  void set_cache_tidy_config(cache_tidy_config const& cfg) {
+    return impl_->set_cache_tidy_config(cfg);
+  }
 
   class impl {
    public:
@@ -188,6 +192,7 @@ class filesystem_v2 {
     readv(uint32_t inode, size_t size, off_t offset) const = 0;
     virtual std::optional<folly::ByteRange> header() const = 0;
     virtual void set_num_workers(size_t num) = 0;
+    virtual void set_cache_tidy_config(cache_tidy_config const& cfg) = 0;
   };
 
  private:

include/dwarfs/inode_reader_v2.h

@@ -34,6 +34,7 @@
 
 namespace dwarfs {
 
+struct cache_tidy_config;
 class block_cache;
 class logger;
 struct iovec_read_buf;
@@ -67,6 +68,10 @@ class inode_reader_v2 {
 
   void set_num_workers(size_t num) { impl_->set_num_workers(num); }
 
+  void set_cache_tidy_config(cache_tidy_config const& cfg) {
+    impl_->set_cache_tidy_config(cfg);
+  }
+
   class impl {
    public:
     virtual ~impl() = default;
@@ -80,6 +85,7 @@ class inode_reader_v2 {
     virtual void dump(std::ostream& os, const std::string& indent,
                       chunk_range chunks) const = 0;
     virtual void set_num_workers(size_t num) = 0;
+    virtual void set_cache_tidy_config(cache_tidy_config const& cfg) = 0;
   };
 
  private:

include/dwarfs/options.h

@@ -40,9 +40,12 @@ struct block_cache_options {
   double decompress_ratio{1.0};
   bool mm_release{true};
   bool init_workers{true};
-  cache_tidy_strategy tidy_strategy{cache_tidy_strategy::NONE};
+};
-  std::chrono::milliseconds tidy_interval;
+
-  std::chrono::milliseconds tidy_expiry_time;
+struct cache_tidy_config {
+  cache_tidy_strategy strategy{cache_tidy_strategy::NONE};
+  std::chrono::milliseconds interval;
+  std::chrono::milliseconds expiry_time;
 };
 
 struct metadata_options {

src/dwarfs.cpp

@@ -132,6 +132,14 @@ void op_init(void* data, struct fuse_conn_info* /*conn*/) {
 
   // we must do this *after* the fuse driver has forked into background
   userdata->fs.set_num_workers(userdata->opts.workers);
+
+  cache_tidy_config tidy;
+  tidy.strategy = userdata->opts.block_cache_tidy_strategy;
+  tidy.interval = userdata->opts.block_cache_tidy_interval;
+  tidy.expiry_time = userdata->opts.block_cache_tidy_max_age;
+
+  // we must do this *after* the fuse driver has forked into background
+  userdata->fs.set_cache_tidy_config(tidy);
 }
 
 template <typename LoggerPolicy>
@@ -646,9 +654,6 @@ void load_filesystem(dwarfs_userdata& userdata) {
   fsopts.block_cache.decompress_ratio = opts.decompress_ratio;
   fsopts.block_cache.mm_release = !opts.cache_image;
   fsopts.block_cache.init_workers = false;
-  fsopts.block_cache.tidy_strategy = opts.block_cache_tidy_strategy;
-  fsopts.block_cache.tidy_interval = opts.block_cache_tidy_interval;
-  fsopts.block_cache.tidy_expiry_time = opts.block_cache_tidy_max_age;
   fsopts.metadata.enable_nlink = bool(opts.enable_nlink);
   fsopts.metadata.readonly = bool(opts.readonly);
 

src/dwarfs/block_cache.cpp

@@ -237,23 +237,13 @@ class block_cache_ final : public block_cache::impl {
                                       : std::thread::hardware_concurrency(),
                                   static_cast<size_t>(1)));
     }
-
-    if (options.tidy_strategy != cache_tidy_strategy::NONE) {
-      tidy_running_ = true;
-      tidy_thread_ = std::thread(&block_cache_::tidy_thread, this);
-    }
   }
 
   ~block_cache_() noexcept override {
     LOG_DEBUG << "stopping cache workers";
 
     if (tidy_running_) {
-      {
+      stop_tidy_thread();
-        std::lock_guard lock(mx_);
-        tidy_running_ = false;
-      }
-      tidy_cond_.notify_all();
-      tidy_thread_.join();
     }
 
     if (wg_) {
@@ -347,6 +337,25 @@ class block_cache_ final : public block_cache::impl {
     wg_ = worker_group("blkcache", num);
   }
 
+  void set_tidy_config(cache_tidy_config const& cfg) override {
+    if (cfg.strategy == cache_tidy_strategy::NONE) {
+      if (tidy_running_) {
+        stop_tidy_thread();
+      }
+    } else {
+      std::lock_guard lock(mx_);
+
+      tidy_config_ = cfg;
+
+      if (tidy_running_) {
+        tidy_cond_.notify_all();
+      } else {
+        tidy_running_ = true;
+        tidy_thread_ = std::thread(&block_cache_::tidy_thread, this);
+      }
+    }
+  }
+
   std::future<block_range>
   get(size_t block_no, size_t offset, size_t size) const override {
     ++range_requests_;
@@ -492,6 +501,15 @@ class block_cache_ final : public block_cache::impl {
   }
 
  private:
+  void stop_tidy_thread() {
+    {
+      std::lock_guard lock(mx_);
+      tidy_running_ = false;
+    }
+    tidy_cond_.notify_all();
+    tidy_thread_.join();
+  }
+
   void update_block_stats(cached_block const& cb) {
     if (cb.range_end() < cb.uncompressed_size()) {
       ++partially_decompressed_;
@@ -582,15 +600,16 @@ class block_cache_ final : public block_cache::impl {
       }
     }
 
-    if (options_.tidy_strategy == cache_tidy_strategy::EXPIRY_TIME) {
-      block->touch();
-    }
-
     // Finally, put the block into the cache; it might already be
     // in there, in which case we just promote it to the front of
     // the LRU queue.
     {
       std::lock_guard lock(mx_);
+
+      if (tidy_config_.strategy == cache_tidy_strategy::EXPIRY_TIME) {
+        block->touch();
+      }
+
       cache_.set(block_no, std::move(block));
     }
   }
@@ -615,13 +634,13 @@ class block_cache_ final : public block_cache::impl {
     std::unique_lock lock(mx_);
 
     while (tidy_running_) {
-      if (tidy_cond_.wait_for(lock, options_.tidy_interval) ==
+      if (tidy_cond_.wait_for(lock, tidy_config_.interval) ==
           std::cv_status::timeout) {
-        switch (options_.tidy_strategy) {
+        switch (tidy_config_.strategy) {
         case cache_tidy_strategy::EXPIRY_TIME:
           tidy_collect(
               [tp = std::chrono::steady_clock::now() -
-                    options_.tidy_expiry_time](cached_block const& blk) {
+                    tidy_config_.expiry_time](cached_block const& blk) {
                 return blk.last_used_before(tp);
               });
           break;
@@ -675,6 +694,7 @@ class block_cache_ final : public block_cache::impl {
   std::shared_ptr<mmif> mm_;
   LOG_PROXY_DECL(LoggerPolicy);
   const block_cache_options options_;
+  cache_tidy_config tidy_config_;
 };
 
 block_cache::block_cache(logger& lgr, std::shared_ptr<mmif> mm,

src/dwarfs/filesystem_v2.cpp

@@ -350,6 +350,9 @@ class filesystem_ final : public filesystem_v2::impl {
   readv(uint32_t inode, size_t size, off_t offset) const override;
   std::optional<folly::ByteRange> header() const override;
   void set_num_workers(size_t num) override { ir_.set_num_workers(num); }
+  void set_cache_tidy_config(cache_tidy_config const& cfg) override {
+    ir_.set_cache_tidy_config(cfg);
+  }
 
  private:
   filesystem_info const& get_info() const;

src/dwarfs/inode_reader_v2.cpp

@@ -69,6 +69,9 @@ class inode_reader_ final : public inode_reader_v2::impl {
   void dump(std::ostream& os, const std::string& indent,
             chunk_range chunks) const override;
   void set_num_workers(size_t num) override { cache_.set_num_workers(num); }
+  void set_cache_tidy_config(cache_tidy_config const& cfg) override {
+    cache_.set_tidy_config(cfg);
+  }
 
  private:
   folly::Expected<std::vector<std::future<block_range>>, int>