update FuseChannel to signal the session complete future immediately

Summary:
This changes FuseChannel to fulfill the session complete future immediately in
the thread where it finishes.  This may occur inside a FUSE worker thread.
It is now up to the caller to use `via()` if they want their callback to run in
a separate thread.

This simplifies the FuseChannel code and also addresses a crash that occurs in
the unit tests sometimes: If the FuseChannel destructor is invoked without
explicitly invoking the FuseChannel, it would schedule the session complete
promise to be fulfilled in a separate EventBase thread.  However, the promise
may then have already been destroyed by the time it can run in the EventBase
thread.

There are still additional synchronization issues to fix in the FuseChannel
tests, but this resolves one problem.

Reviewed By: chadaustin

Differential Revision: D7282001

fbshipit-source-id: be64d3ef6a0e664ed7a2cf93a549acc140184095

eden/fs/fuse/FuseChannel.cpp

@@ -11,7 +11,6 @@
 
 #include <folly/experimental/logging/xlog.h>
 #include <folly/futures/helpers.h>
-#include <folly/io/async/EventBase.h>
 #include <folly/io/async/Request.h>
 #include <folly/system/ThreadName.h>
 #include <signal.h>
@@ -313,13 +312,11 @@ void FuseChannel::sendRawReply(const iovec iov[], size_t count) const {
 FuseChannel::FuseChannel(
     folly::File&& fuseDevice,
     AbsolutePathPiece mountPath,
-    folly::EventBase* eventBase,
     size_t numThreads,
     Dispatcher* const dispatcher)
     : bufferSize_(std::max(size_t(getpagesize()) + 0x1000, MIN_BUFSIZE)),
       numThreads_(numThreads),
       dispatcher_(dispatcher),
-      eventBase_(eventBase),
       mountPath_(mountPath),
       fuseDevice_(std::move(fuseDevice)) {
   CHECK_GE(numThreads_, 1);
@@ -570,10 +567,7 @@ void FuseChannel::fuseWorkerThread() noexcept {
   }
 
   if (complete) {
-    eventBase_->runInEventBaseThread([this]() {
-      sessionCompletePromise_.setValue(
-          runState_.load(std::memory_order_relaxed));
-    });
+    sessionCompletePromise_.setValue(runState_.load(std::memory_order_relaxed));
   }
 }
 

eden/fs/fuse/FuseChannel.h

@@ -25,7 +25,6 @@
 
 namespace folly {
 class RequestContext;
-class EventBase;
 } // namespace folly
 
 namespace facebook {
@@ -62,7 +61,6 @@ class FuseChannel {
   FuseChannel(
       folly::File&& fuseDevice,
       AbsolutePathPiece mountPath,
-      folly::EventBase* eventBase,
       size_t numThreads,
       Dispatcher* const dispatcher);
 
@@ -221,15 +219,20 @@ class FuseChannel {
   void finishRequest(const fuse_in_header& header);
 
   /**
-   * Returns a Future that will complete when all of the
-   * fuse threads have been joined and when all pending
-   * fuse requests initiated by the kernel have been
-   * responded to.
+   * Returns a SemiFuture that will complete when all of the fuse threads have
+   * been joined and when all pending fuse requests initiated by the kernel
+   * have been responded to.
    *
    * Will throw if called more than once.
    *
    * The session completion future will only be signaled if initialization
    * (via initialize() or takeoverInitialize()) has completed successfully.
+   *
+   * This future may be fulfilled inside one of the FUSE worker threads that is
+   * in the process of shutting down.  Therefore is not safe to call
+   * FuseChannel::destroy() directly in the thread where this future is
+   * fulfilled.  Callers should generally use Future::via() to move execution
+   * to another executor thread.
    */
   folly::Future<StopReason> getSessionCompleteFuture();
 
@@ -383,7 +386,6 @@ class FuseChannel {
   const size_t bufferSize_{0};
   const size_t numThreads_;
   Dispatcher* const dispatcher_{nullptr};
-  folly::EventBase* const eventBase_;
   const AbsolutePath mountPath_;
 
   /*

eden/fs/fuse/test/FuseChannelTest.cpp

@@ -10,7 +10,6 @@
 #include "eden/fs/fuse/FuseChannel.h"
 
 #include <folly/experimental/logging/xlog.h>
-#include <folly/io/async/ScopedEventBaseThread.h>
 #include <folly/test/TestUtils.h>
 #include <gtest/gtest.h>
 #include "eden/fs/fuse/Dispatcher.h"
@@ -22,7 +21,6 @@ using namespace facebook::eden::fusell;
 using namespace std::literals::chrono_literals;
 using folly::ByteRange;
 using folly::Future;
-using folly::ScopedEventBaseThread;
 using folly::Unit;
 using std::make_unique;
 
@@ -33,15 +31,10 @@ class TestDispatcher : public Dispatcher {
 
 class FuseChannelTest : public ::testing::Test {
  protected:
-  void SetUp() override {
-    eventBaseThread_ = make_unique<ScopedEventBaseThread>();
-  }
-
   std::unique_ptr<FuseChannel> createChannel(size_t numThreads = 2) {
     return make_unique<FuseChannel>(
         fuse_.start(),
         mountPath_,
-        eventBaseThread_->getEventBase(),
         numThreads,
         &dispatcher_);
   }
@@ -71,7 +64,6 @@ class FuseChannelTest : public ::testing::Test {
   ThreadLocalEdenStats stats_;
   TestDispatcher dispatcher_{&stats_};
   AbsolutePath mountPath_{"/fake/mount/path"};
-  std::unique_ptr<ScopedEventBaseThread> eventBaseThread_;
 };
 
 } // namespace

eden/fs/inodes/EdenMount.cpp

@@ -714,13 +714,10 @@ void EdenMount::createFuseChannel(
   threadPool_ = std::move(threadPool);
 
   channel_ = std::make_unique<fusell::FuseChannel>(
-      std::move(fuseDevice),
-      path_,
-      eventBase_,
-      FLAGS_fuseNumThreads,
-      dispatcher_.get());
+      std::move(fuseDevice), path_, FLAGS_fuseNumThreads, dispatcher_.get());
 
   channel_->getSessionCompleteFuture()
+      .via(eventBase_)
       .then([this] {
         // In case we are performing a graceful restart,
         // extract the fuse device now.