inodes: perform maintenance in the overlay thread

Summary:
During shutdown, the overlay background thread is terminated, and the overlay
closed, but EdenServer timers are still running. One of which being the
manageOverlay one which performs a maintenance on the Overlay. In some cases,
shutdown and this timer are racing each other, causing the timer to run after
the overlay has been closed, causing a use after free and crashing EdenFS.

To solve this, we can either add locks around closing the overlay and the
maintenance to guarantee that they do not race, or we can move the maintenance
operation to a thread that is known to be running only when the overlay is
opened. This diff takes the second approach.

The bug manifest itself like so:

  I0712 01:16:48.253296 21620 EdenServiceHandler.cpp:3394] [000002517432E1E0] initiateShutdown() took 368 µs
  I0712 01:16:48.253838 24700 PrjfsChannel.cpp:1185] Stopping PrjfsChannel for: C:\\cygwin\\tmp\\eden_test.stop_at_ea.m3931hyz\\mounts\\main
  I0712 01:16:48.258533 19188 EdenServer.cpp:1624] mount point "C:\\cygwin\\tmp\\eden_test.stop_at_ea.m3931hyz\\mounts\\main" stopped
  V0712 01:16:48.258814 19188 EdenMount.cpp:851] beginning shutdown for EdenMount C:\\cygwin\\tmp\\eden_test.stop_at_ea.m3931hyz\\mounts\\main
  V0712 01:16:48.259895 19188 EdenMount.cpp:855] shutdown complete for EdenMount C:\\cygwin\\tmp\\eden_test.stop_at_ea.m3931hyz\\mounts\\main
  V0712 01:16:48.287378 19188 EdenMount.cpp:861] successfully closed overlay at C:\\cygwin\\tmp\\eden_test.stop_at_ea.m3931hyz\\mounts\\main
  Unhandled win32 exception code=0xC0000005. Fatal error detected at:
  #0 00007FF707BA3D81 (7a686d9) facebook::eden::SqliteDatabase::checkpoint Z:\shipit\eden\eden\fs\sqlite\SqliteDatabase.cpp:99
  #1 00007FF7072D4090 facebook::eden::EdenServer::manageOverlay Z:\shipit\eden\eden\fs\service\EdenServer.cpp:2142
  #2 00007FF7074765D0 facebook::eden::PeriodicTask::timeoutExpired Z:\shipit\eden\eden\fs\service\PeriodicTask.cpp:32
  #3 00007FF707500F93 folly::HHWheelTimerBase<std::chrono::duration<__int64,std::ratio<1,1000> > >::timeoutExpired Z:\shipit\folly\folly\io\async\HHWheelTimer.cpp:286
  #4 00007FF70757BB44 folly::AsyncTimeout::libeventCallback Z:\shipit\folly\folly\io\async\AsyncTimeout.cpp:174
  #5 00007FF708E7AD45 (645b6fc) event_priority_set
  #6 00007FF708E7AA3A event_priority_set
  #7 00007FF708E77343 event_base_loop
  #8 00007FF707515FC5 folly::EventBase::loopMain Z:\shipit\folly\folly\io\async\EventBase.cpp:405
  #9 00007FF707515C62 folly::EventBase::loopBody Z:\shipit\folly\folly\io\async\EventBase.cpp:326
  #10 00007FF7072DC5EE facebook::eden::EdenServer::performCleanup Z:\shipit\eden\eden\fs\service\EdenServer.cpp:1212
  #11 00007FF707219BED facebook::eden::runEdenMain Z:\shipit\eden\eden\fs\service\EdenMain.cpp:395
  #12 00007FF7071C624A main Z:\shipit\eden\eden\fs\service\oss\main.cpp:23
  #13 00007FF708E87A94 __scrt_common_main_seh d:\A01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
  #14 00007FFC96DC7034 BaseThreadInitThunk
  #15 00007FFC98C5CEC1 RtlUserThreadStart

Reviewed By: fanzeyi

Differential Revision: D37793444

fbshipit-source-id: cd33302789c2c7a29d566d5bac6e119eccf0a5f2

Author: xavierd

eden/fs/inodes/Overlay.cpp

@@ -531,8 +531,16 @@ void Overlay::gcThread() noexcept {
 
 void Overlay::handleGCRequest(GCRequest& request) {
   IORequest req{this};
-  if (request.flush) {
-    request.flush->setValue();
+
+  if (std::holds_alternative<GCRequest::MaintenanceRequest>(
+          request.requestType)) {
+    backingOverlay_->maintenance();
+    return;
+  }
+
+  if (auto* flush =
+          std::get_if<GCRequest::FlushRequest>(&request.requestType)) {
+    flush->setValue();
     return;
   }
 
@@ -573,7 +581,7 @@ void Overlay::handleGCRequest(GCRequest& request) {
     }
   };
 
-  processDir(request.dir);
+  processDir(std::get<overlay::OverlayDir>(request.requestType));
 
   while (!queue.empty()) {
     auto ino = queue.front();
@@ -644,6 +652,6 @@ void Overlay::renameChild(
 }
 
 void Overlay::maintenance() {
-  backingOverlay_->maintenance();
+  gcQueue_.lock()->queue.emplace_back(Overlay::GCRequest::MaintenanceRequest{});
 }
 } // namespace facebook::eden

eden/fs/inodes/Overlay.h

@@ -267,24 +267,27 @@ class Overlay : public std::enable_shared_from_this<Overlay> {
       std::shared_ptr<StructuredLogger> logger);
 
   /**
-   * A request for the background GC thread.  There are two types of requests:
-   * recursively forget data underneath an given directory, or complete a
-   * promise.  The latter is used for synchronization with the GC thread,
-   * primarily in unit tests.
+   * A request for the background GC thread.  There are three types of
+   * requests: recursively forget data underneath a given directory, perform
+   * some maintenance of the overlay or complete a promise.  The latter is used
+   * for synchronization with the GC thread, primarily in unit tests.
    *
    * If additional request types are added in the future, consider renaming to
    * AsyncRequest.  However, recursive collection of forgotten inode numbers
    * is the only operation that can be made async while preserving our
    * durability goals.
    */
   struct GCRequest {
-    GCRequest() {}
-    explicit GCRequest(overlay::OverlayDir&& d) : dir{std::move(d)} {}
-    explicit GCRequest(folly::Promise<folly::Unit> p) : flush{std::move(p)} {}
+    explicit GCRequest(overlay::OverlayDir&& d) : requestType{std::move(d)} {}
 
-    overlay::OverlayDir dir;
-    // Iff set, this is a flush request.
-    std::optional<folly::Promise<folly::Unit>> flush;
+    using FlushRequest = folly::Promise<folly::Unit>;
+    explicit GCRequest(FlushRequest p) : requestType{std::move(p)} {}
+
+    struct MaintenanceRequest {};
+    explicit GCRequest(MaintenanceRequest req) : requestType{std::move(req)} {}
+
+    std::variant<MaintenanceRequest, overlay::OverlayDir, FlushRequest>
+        requestType;
   };
 
   struct GCQueue {