Remove AtFork handling logic from RCU

Summary:
folly RCU currently has a somewhat awkward restriction that there may be no RCU
readers across a fork() call, and that after fork() has been invoked, no AtFork
handlers may enter a read region *unless* they spawn a separate thread in the
AtFork handler. In the child's fork() handler, we then reset the forked child
thread's TLS read counters, and even have a ForkTest that validates that if we
were in a read region while calling fork(), that the child's state is wiped out.

The fact that we zero out the forked child thread's readers rather than
explicitly just panicking is very confusing, and does not match the advertised
behavior of the RCU subsystem. The only thing that's really valid to do in if
fork() is involved is:

1. Have a single threaded program that is not in a read region.
2. Have a single threaded program that is in a read region, and immediately
   invokes exec().

These restrictions are common to most librcu implementations. For folly RCU, we
can't do anything to verify that the program was single threaded when we get to
an AtFork handler, and we also can't check that if the caller is in a read
region that they will soon exec(). What we definitely should *not* do is
instill some artificial state of clearing out the read region of the forking
thread on a fork() invocation. This change therefore just removes the folly
AtFork handling logic.

Reviewed By: paulmckrcu

Differential Revision: D35557080

fbshipit-source-id: 77e3bdaaea078c60e7f7c562228de08635f164e6

Author: Byte-Lab

folly/synchronization/Rcu-inl.h

@@ -16,7 +16,6 @@
 
 #include <folly/Function.h>
 #include <folly/detail/TurnSequencer.h>
-#include <folly/system/AtFork.h>
 
 namespace folly {
 
@@ -29,24 +28,6 @@ rcu_domain<Tag>::rcu_domain(Executor* executor) noexcept
   // Please use a unique tag for each domain.
   CHECK(!singleton_);
   singleton_ = true;
-
-  // Register fork handlers.  Holding read locks across fork is not
-  // supported.  Using read locks in other atfork handlers is not
-  // supported.  Other atfork handlers launching new child threads
-  // that use read locks *is* supported.
-  AtFork::registerHandler(
-      this,
-      [this]() { return syncMutex_.try_lock(); },
-      [this]() { syncMutex_.unlock(); },
-      [this]() {
-        counters_.resetAfterFork();
-        syncMutex_.unlock();
-      });
-}
-
-template <typename Tag>
-rcu_domain<Tag>::~rcu_domain() {
-  AtFork::unregisterHandler(this);
 }
 
 template <typename Tag>

folly/synchronization/Rcu.h

@@ -188,6 +188,15 @@
 //   the default executor type.
 
 // API correctness limitations:
+//
+//  - fork():
+//    Invoking fork() in a multithreaded program with any thread other than the
+//    forking thread being present in a read region will result in undefined
+//    behavior. Similarly, a forking thread must immediately invoke exec if
+//    fork() is invoked while in a read region. Invoking fork() inside of a read
+//    region, and then exiting before invoking exec(), will similarly result in
+//    undefined behavior.
+//
 //  - Exceptions:
 //    In short, nothing about this is exception safe. retire functions should
 //    not throw exceptions in their destructors, move constructors or call
@@ -328,7 +337,6 @@ class rcu_domain {
   rcu_domain(rcu_domain&&) = delete;
   rcu_domain& operator=(const rcu_domain&) = delete;
   rcu_domain& operator=(rcu_domain&&) = delete;
-  ~rcu_domain();
 
   // Reader locks: Prevent any calls from occuring, retired memory
   // from being freed, and synchronize() calls from completing until

folly/synchronization/detail/ThreadCachedReaders.h

@@ -186,17 +186,6 @@ class ThreadCachedReaders {
     }
     waiting_.store(0, std::memory_order_relaxed);
   }
-
-  // We are guaranteed to be called while StaticMeta lock is still
-  // held because of ordering in AtForkList.  We can therefore safely
-  // touch orphan_ and clear out all counts.
-  void resetAfterFork() {
-    if (tls_cache_) {
-      tls_cache_->epoch_readers_ = 0;
-    }
-    orphan_epoch_readers_ = 0;
-    folly::asymmetricLightBarrier();
-  }
 };
 
 template <typename Tag>

folly/synchronization/test/RcuTest.cpp

@@ -197,21 +197,24 @@ TEST(RcuTest, SynchronizeInCall) {
   rcu_synchronize();
 }
 
-TEST(RcuTest, ForkTest) {
+TEST(RcuTest, SafeForkTest) {
   rcu_default_domain().lock();
+  rcu_default_domain().unlock();
   auto pid = fork();
-  if (pid) {
-    // parent
-    rcu_default_domain().unlock();
+  if (pid > 0) {
+    // Parent branch -- wait for child to exit.
     rcu_synchronize();
     int status = -1;
     auto pid2 = waitpid(pid, &status, 0);
-    EXPECT_EQ(status, 0);
     EXPECT_EQ(pid, pid2);
-  } else {
-    // child
+    EXPECT_EQ(status, 0);
+  } else if (pid == 0) {
     rcu_synchronize();
-    exit(0); // Do not print gtest results
+    // Exit quickly to avoid spamming gtest output to console.
+    exit(0);
+  } else {
+    // Skip the test if fork() fails.
+    GTEST_SKIP();
   }
 }