Fix folly::call_once

Summary: std::call_once implementation is broken if function throws. This fixes folly::call_once to not depend on std::call_once.

Reviewed By: yfeldblum

Differential Revision: D5015897

fbshipit-source-id: bcbda68becf0930cdbf0b09125cbee61d75c2015

diff --git a/folly/CallOnce.h b/folly/CallOnce.h
index 77db62106ff4a85ec72c320b2e0dbbf3fe0a868c..4dd06941a25bb3f584f644f988738d51e9588f3f 100644 (file)
--- a/folly/CallOnce.h
+++ b/folly/CallOnce.h
@@ -36,6 +36,7 @@
 
 #include <folly/Likely.h>
 #include <folly/Portability.h>
+#include <folly/SharedMutex.h>
 
 namespace folly {
 
@@ -54,7 +55,7 @@ class once_flag {
 
  private:
   std::atomic<bool> called_{false};
-  std::once_flag std_once_flag_;
+  folly::SharedMutex mutex_;
 };
 
 template <class Callable, class... Args>
@@ -71,9 +72,13 @@ call_once(once_flag& flag, Callable&& f, Args&&... args) {
 template <class Callable, class... Args>
 void FOLLY_NOINLINE
 call_once_impl_no_inline(once_flag& flag, Callable&& f, Args&&... args) {
-  std::call_once(flag.std_once_flag_,
-                 std::forward<Callable>(f),
-                 std::forward<Args>(args)...);
+  std::lock_guard<folly::SharedMutex> lg(flag.mutex_);
+  if (flag.called_) {
+    return;
+  }
+
+  std::forward<Callable>(f)(std::forward<Args>(args)...);
+
   flag.called_.store(true, std::memory_order_release);
 }
 }

diff --git a/folly/SharedMutex.h b/folly/SharedMutex.h
index 571af4a7e2749342c5b1a9ea811271bc550ac6a7..c13a6d6f17ad27cf517c0fca13f602f178bf080d 100644 (file)
--- a/folly/SharedMutex.h
+++ b/folly/SharedMutex.h
@@ -249,7 +249,7 @@ class SharedMutexImpl {
   class UpgradeHolder;
   class WriteHolder;
 
-  constexpr SharedMutexImpl() : state_(0) {}
+  constexpr SharedMutexImpl() noexcept : state_(0) {}
 
   SharedMutexImpl(const SharedMutexImpl&) = delete;
   SharedMutexImpl(SharedMutexImpl&&) = delete;

diff --git a/folly/detail/Futex.cpp b/folly/detail/Futex.cpp
index fa628602aaa92bf32e1405e3c371cb2110d06f26..edaa279e77ffb2eaccc31eeed8bfe7451d57b8f4 100644 (file)
--- a/folly/detail/Futex.cpp
+++ b/folly/detail/Futex.cpp
@@ -20,7 +20,6 @@
 #include <condition_variable>
 #include <mutex>
 #include <boost/intrusive/list.hpp>
-#include <folly/CallOnce.h>
 #include <folly/Hash.h>
 #include <folly/ScopeGuard.h>
 
@@ -187,22 +186,15 @@ struct EmulatedFutexBucket {
   boost::intrusive::list<EmulatedFutexWaitNode> waiters_;
 
   static const size_t kNumBuckets = 4096;
-  static EmulatedFutexBucket* gBuckets;
-  static folly::once_flag gBucketInit;
 
   static EmulatedFutexBucket& bucketFor(void* addr) {
-    folly::call_once(gBucketInit, [](){
-      gBuckets = new EmulatedFutexBucket[kNumBuckets];
-    });
+    static auto gBuckets = new EmulatedFutexBucket[kNumBuckets];
     uint64_t mixedBits = folly::hash::twang_mix64(
         reinterpret_cast<uintptr_t>(addr));
     return gBuckets[mixedBits % kNumBuckets];
   }
 };
 
-EmulatedFutexBucket* EmulatedFutexBucket::gBuckets;
-folly::once_flag EmulatedFutexBucket::gBucketInit;
-
 int emulatedFutexWake(void* addr, int count, uint32_t waitMask) {
   auto& bucket = EmulatedFutexBucket::bucketFor(addr);
   std::unique_lock<std::mutex> bucketLock(bucket.mutex_);

diff --git a/folly/test/CallOnceTest.cpp b/folly/test/CallOnceTest.cpp
index 357b697397e2b5a578dfb2d4f18c89fa44f717ff..e0dccd81f87684fa84c2eea81994bac7d49f1eae 100644 (file)
--- a/folly/test/CallOnceTest.cpp
+++ b/folly/test/CallOnceTest.cpp
@@ -50,6 +50,23 @@ TEST(FollyCallOnce, Simple) {
   ASSERT_EQ(1, out);
 }
 
+TEST(FollyCallOnce, Exception) {
+  struct ExpectedException {};
+  folly::once_flag flag;
+  size_t numCalls = 0;
+  EXPECT_THROW(
+      folly::call_once(
+          flag,
+          [&] {
+            ++numCalls;
+            throw ExpectedException();
+          }),
+      ExpectedException);
+  EXPECT_EQ(1, numCalls);
+  folly::call_once(flag, [&] { ++numCalls; });
+  EXPECT_EQ(2, numCalls);
+}
+
 TEST(FollyCallOnce, Stress) {
   for (int i = 0; i < 100; ++i) {
     folly::once_flag flag;