never destroy LifoSem's wait node pool

Summary: Some LifoSem-s survive late into program execution, which means
that destroying the IndexedMemPool that holds LifoSem waiter nodes can
cause crashes during program shutdown.

Reviewed By: @chadparry

Differential Revision: D2536597

fb-gh-sync-id: c9b3b73b61f2b8bdcb00d03f4c6d3daf24b267c3

diff --git a/folly/LifoSem.h b/folly/LifoSem.h
index b0af517c68b0f9047a766243a8f30eeab6363b9c..4aa1cc888209479a49d66617e43222a6c8aec8f7 100644 (file)
--- a/folly/LifoSem.h
+++ b/folly/LifoSem.h
@@ -131,15 +131,21 @@ struct LifoSemRawNode {
   typedef folly::IndexedMemPool<LifoSemRawNode<Atom>,32,200,Atom> Pool;
 
   /// Storage for all of the waiter nodes for LifoSem-s that use Atom
-  static Pool pool;
+  static Pool& pool();
 };
 
 /// Use this macro to declare the static storage that backs the raw nodes
 /// for the specified atomic type
-#define LIFOSEM_DECLARE_POOL(Atom, capacity)                       \
-    template<>                                                     \
-    folly::detail::LifoSemRawNode<Atom>::Pool                      \
-        folly::detail::LifoSemRawNode<Atom>::pool((capacity));
+#define LIFOSEM_DECLARE_POOL(Atom, capacity)                 \
+  namespace folly {                                          \
+  namespace detail {                                         \
+  template <>                                                \
+  LifoSemRawNode<Atom>::Pool& LifoSemRawNode<Atom>::pool() { \
+    static Pool* instance = new Pool((capacity));            \
+    return *instance;                                        \
+  }                                                          \
+  }                                                          \
+  }
 
 /// Handoff is a type not bigger than a void* that knows how to perform a
 /// single post() -> wait() communication.  It must have a post() method.
@@ -180,8 +186,8 @@ template <typename Handoff, template<typename> class Atom>
 struct LifoSemNodeRecycler {
   void operator()(LifoSemNode<Handoff,Atom>* elem) const {
     elem->destroy();
-    auto idx = LifoSemRawNode<Atom>::pool.locateElem(elem);
-    LifoSemRawNode<Atom>::pool.recycleIndex(idx);
+    auto idx = LifoSemRawNode<Atom>::pool().locateElem(elem);
+    LifoSemRawNode<Atom>::pool().recycleIndex(idx);
   }
 };
 
@@ -478,14 +484,14 @@ struct LifoSemBase {
   /// Returns a node that can be passed to decrOrLink
   template <typename... Args>
   UniquePtr allocateNode(Args&&... args) {
-    auto idx = LifoSemRawNode<Atom>::pool.allocIndex();
+    auto idx = LifoSemRawNode<Atom>::pool().allocIndex();
     if (idx != 0) {
       auto& node = idxToNode(idx);
       node.clearShutdownNotice();
       try {
         node.init(std::forward<Args>(args)...);
       } catch (...) {
-        LifoSemRawNode<Atom>::pool.recycleIndex(idx);
+        LifoSemRawNode<Atom>::pool().recycleIndex(idx);
         throw;
       }
       return UniquePtr(&node);
@@ -515,12 +521,12 @@ struct LifoSemBase {
 
 
   static LifoSemNode<Handoff, Atom>& idxToNode(uint32_t idx) {
-    auto raw = &LifoSemRawNode<Atom>::pool[idx];
+    auto raw = &LifoSemRawNode<Atom>::pool()[idx];
     return *static_cast<LifoSemNode<Handoff, Atom>*>(raw);
   }
 
   static uint32_t nodeToIdx(const LifoSemNode<Handoff, Atom>& node) {
-    return LifoSemRawNode<Atom>::pool.locateElem(&node);
+    return LifoSemRawNode<Atom>::pool().locateElem(&node);
   }
 
   /// Either increments by n and returns 0, or pops a node and returns it.

diff --git a/folly/test/LifoSemTests.cpp b/folly/test/LifoSemTests.cpp
index 85e67c8bae056419e92078b431057730b0e6f2e8..6006c10da30d546f031c9e881a98716506b3b358 100644 (file)
--- a/folly/test/LifoSemTests.cpp
+++ b/folly/test/LifoSemTests.cpp
@@ -401,31 +401,40 @@ static void contendedUse(uint n, int posters, int waiters) {
 
 BENCHMARK_DRAW_LINE()
 BENCHMARK_NAMED_PARAM(contendedUse, 1_to_1, 1, 1)
+BENCHMARK_NAMED_PARAM(contendedUse, 1_to_4, 1, 4)
 BENCHMARK_NAMED_PARAM(contendedUse, 1_to_32, 1, 32)
+BENCHMARK_NAMED_PARAM(contendedUse, 4_to_1, 4, 1)
+BENCHMARK_NAMED_PARAM(contendedUse, 4_to_24, 4, 24)
+BENCHMARK_NAMED_PARAM(contendedUse, 8_to_100, 8, 100)
 BENCHMARK_NAMED_PARAM(contendedUse, 32_to_1, 31, 1)
 BENCHMARK_NAMED_PARAM(contendedUse, 16_to_16, 16, 16)
 BENCHMARK_NAMED_PARAM(contendedUse, 32_to_32, 32, 32)
 BENCHMARK_NAMED_PARAM(contendedUse, 32_to_1000, 32, 1000)
 
-// sudo nice -n -20 folly/test/LifoSemTests --benchmark --bm_min_iters=10000000
+// sudo nice -n -20 _build/opt/folly/test/LifoSemTests \
+//     --benchmark --bm_min_iters=10000000 --gtest_filter=-\*
 // ============================================================================
 // folly/test/LifoSemTests.cpp                     relative  time/iter  iters/s
 // ============================================================================
-// lifo_sem_pingpong                                          396.84ns    2.52M
-// lifo_sem_oneway                                             88.52ns   11.30M
-// single_thread_lifo_post                                     14.78ns   67.67M
-// single_thread_lifo_wait                                     13.53ns   73.90M
-// single_thread_lifo_postwait                                 28.91ns   34.59M
-// single_thread_lifo_trywait                                 670.13ps    1.49G
-// single_thread_posix_postwait                                24.12ns   41.46M
-// single_thread_posix_trywait                                  6.76ns  147.88M
+// lifo_sem_pingpong                                            1.31us  762.40K
+// lifo_sem_oneway                                            193.89ns    5.16M
+// single_thread_lifo_post                                     15.37ns   65.08M
+// single_thread_lifo_wait                                     13.60ns   73.53M
+// single_thread_lifo_postwait                                 29.43ns   33.98M
+// single_thread_lifo_trywait                                 677.69ps    1.48G
+// single_thread_posix_postwait                                25.03ns   39.95M
+// single_thread_posix_trywait                                  7.30ns  136.98M
 // ----------------------------------------------------------------------------
-// contendedUse(1_to_1)                                       143.60ns    6.96M
-// contendedUse(1_to_32)                                      244.06ns    4.10M
-// contendedUse(32_to_1)                                      131.99ns    7.58M
-// contendedUse(16_to_16)                                     210.64ns    4.75M
-// contendedUse(32_to_32)                                     222.91ns    4.49M
-// contendedUse(32_to_1000)                                   453.39ns    2.21M
+// contendedUse(1_to_1)                                       158.22ns    6.32M
+// contendedUse(1_to_4)                                       574.73ns    1.74M
+// contendedUse(1_to_32)                                      592.94ns    1.69M
+// contendedUse(4_to_1)                                       118.28ns    8.45M
+// contendedUse(4_to_24)                                      667.62ns    1.50M
+// contendedUse(8_to_100)                                     701.46ns    1.43M
+// contendedUse(32_to_1)                                      165.06ns    6.06M
+// contendedUse(16_to_16)                                     238.57ns    4.19M
+// contendedUse(32_to_32)                                     219.82ns    4.55M
+// contendedUse(32_to_1000)                                   777.42ns    1.29M
 // ============================================================================
 
 int main(int argc, char ** argv) {