Merge pull request #200 from boostorg/feature/detect_reentrancy

Feature/detect reentrancy

doc/unordered/changes.adoc

@@ -12,6 +12,8 @@
 with serial and parallel variants.
 * Added efficient move construction of `boost::unordered_flat_map` from
 `boost::concurrent_flat_map` and vice versa.
+* Added debug mode mechanisms for detecting illegal reentrancies into
+a `boost::concurrent_flat_map` from user code.
 
 == Release 1.83.0 - Major update
 

doc/unordered/concurrent_flat_map.adoc

@@ -375,6 +375,18 @@ if concurrent outstanding operations on `y` do not access `x` directly or indire
 
 ---
 
+=== Configuration Macros
+
+==== `BOOST_UNORDERED_DISABLE_REENTRANCY_CHECK`
+
+In debug builds (more precisely, when
+link:../../../assert/doc/html/assert.html#boost_assert_is_void[`BOOST_ASSERT_IS_VOID`^]
+is not defined), __container reentrancies__ (illegaly invoking an operation on `m` from within
+a function visiting elements of `m`) are detected and signalled through `BOOST_ASSERT_MSG`.
+When run-time speed is a concern, the feature can be disabled by globally defining
+this macro.
+
+
 === Constructors
 
 ==== Default Constructor

include/boost/unordered/detail/foa/concurrent_table.hpp

@@ -20,6 +20,7 @@
 #include <boost/mp11/tuple.hpp>
 #include <boost/static_assert.hpp>
 #include <boost/unordered/detail/foa/core.hpp>
+#include <boost/unordered/detail/foa/reentrancy_check.hpp>
 #include <boost/unordered/detail/foa/rw_spinlock.hpp>
 #include <boost/unordered/detail/foa/tuple_rotate_right.hpp>
 #include <cstddef>
@@ -910,9 +911,9 @@ private:
 
   using mutex_type=rw_spinlock;
   using multimutex_type=multimutex<mutex_type,128>; // TODO: adapt 128 to the machine
-  using shared_lock_guard=shared_lock<mutex_type>;
-  using exclusive_lock_guard=lock_guard<multimutex_type>;
-  using exclusive_bilock_guard=scoped_bilock<multimutex_type>;
+  using shared_lock_guard=reentrancy_checked<shared_lock<mutex_type>>;
+  using exclusive_lock_guard=reentrancy_checked<lock_guard<multimutex_type>>;
+  using exclusive_bilock_guard=reentrancy_bichecked<scoped_bilock<multimutex_type>>;
   using group_shared_lock_guard=typename group_access::shared_lock_guard;
   using group_exclusive_lock_guard=typename group_access::exclusive_lock_guard;
   using group_insert_counter_type=typename group_access::insert_counter_type;
@@ -932,18 +933,18 @@ private:
   {
     thread_local auto id=(++thread_counter)%mutexes.size();
 
-    return shared_lock_guard{mutexes[id]};
+    return shared_lock_guard{this,mutexes[id]};
   }
 
   inline exclusive_lock_guard exclusive_access()const
   {
-    return exclusive_lock_guard{mutexes};
+    return exclusive_lock_guard{this,mutexes};
   }
 
   static inline exclusive_bilock_guard exclusive_access(
     const concurrent_table& x,const concurrent_table& y)
   {
-    return {x.mutexes,y.mutexes};
+    return {&x,&y,x.mutexes,y.mutexes};
   }
 
   template<typename Hash2,typename Pred2>
@@ -951,7 +952,7 @@ private:
     const concurrent_table& x,
     const concurrent_table<TypePolicy,Hash2,Pred2,Allocator>& y)
   {
-    return {x.mutexes,y.mutexes};
+    return {&x,&y,x.mutexes,y.mutexes};
   }
 
   /* Tag-dispatched shared/exclusive group access */

include/boost/unordered/detail/foa/reentrancy_check.hpp

@@ -0,0 +1,138 @@
+/* Copyright 2023 Joaquin M Lopez Munoz.
+ * Distributed under the Boost Software License, Version 1.0.
+ * (See accompanying file LICENSE_1_0.txt or copy at
+ * http://www.boost.org/LICENSE_1_0.txt)
+ *
+ * See https://www.boost.org/libs/unordered for library home page.
+ */
+
+#ifndef BOOST_UNORDERED_DETAIL_FOA_REENTRANCY_CHECK_HPP
+#define BOOST_UNORDERED_DETAIL_FOA_REENTRANCY_CHECK_HPP
+
+#include <boost/assert.hpp>
+#include <utility>
+
+#if !defined(BOOST_UNORDERED_DISABLE_REENTRANCY_CHECK)&& \
+    !defined(BOOST_ASSERT_IS_VOID)
+#define BOOST_UNORDERED_REENTRANCY_CHECK
+#endif
+
+namespace boost{
+namespace unordered{
+namespace detail{
+namespace foa{
+
+#if defined(BOOST_UNORDERED_REENTRANCY_CHECK)
+
+class entry_trace
+{
+public:
+  entry_trace(const void* px_):px{px_}
+  {
+    if(px){
+      BOOST_ASSERT_MSG(!find(px),"reentrancy not allowed");
+      header()=this;
+    }
+  }
+
+  /* not used but VS in pre-C++17 mode needs to see it for RVO */
+  entry_trace(const entry_trace&);
+
+  ~entry_trace(){clear();}
+
+  void clear()
+  {
+    if(px){
+      header()=next;
+      px=nullptr;
+    }
+  }
+  
+private:
+  static entry_trace*& header()
+  {
+    thread_local entry_trace *pe=nullptr;
+    return pe;
+  }
+
+  static bool find(const void* px)
+  {
+    for(auto pe=header();pe;pe=pe->next){
+      if(pe->px==px)return true;
+    }
+    return false;
+  }
+
+  const void  *px;
+  entry_trace *next=header();
+};
+
+template<typename LockGuard>
+struct reentrancy_checked
+{
+  template<typename... Args>
+  reentrancy_checked(const void* px,Args&&... args):
+    tr{px},lck{std::forward<Args>(args)...}{}
+
+  void unlock()
+  {
+    lck.unlock();
+    tr.clear();
+  }
+
+  entry_trace tr;
+  LockGuard   lck;
+};
+
+template<typename LockGuard>
+struct reentrancy_bichecked
+{
+  template<typename... Args>
+  reentrancy_bichecked(const void* px,const void* py,Args&&... args):
+    tr1{px},tr2{py!=px?py:nullptr},lck{std::forward<Args>(args)...}{}
+
+  void unlock()
+  {
+    lck.unlock();
+    tr2.clear();
+    tr1.clear();
+  }
+
+  entry_trace tr1,tr2;
+  LockGuard   lck;
+};
+
+#else
+
+template<typename LockGuard>
+struct reentrancy_checked
+{
+  template<typename... Args>
+  reentrancy_checked(const void*,Args&&... args):
+    lck{std::forward<Args>(args)...}{}
+
+  void unlock(){lck.unlock();}
+
+  LockGuard lck;
+};
+
+template<typename LockGuard>
+struct reentrancy_bichecked
+{
+  template<typename... Args>
+  reentrancy_bichecked(const void*,const void*,Args&&... args):
+    lck{std::forward<Args>(args)...}{}
+
+  void unlock(){lck.unlock();}
+
+  LockGuard lck;
+};
+
+#endif
+
+} /* namespace foa */
+} /* namespace detail */
+} /* namespace unordered */
+} /* namespace boost */
+
+#endif

test/Jamfile.v2

@@ -202,6 +202,7 @@ local CFOA_TESTS =
   rw_spinlock_test6
   rw_spinlock_test7
   rw_spinlock_test8
+  reentrancy_check_test
 ;
 
 for local test in $(CFOA_TESTS)

test/cfoa/reentrancy_check_test.cpp

@@ -0,0 +1,79 @@
+// Copyright 2023 Joaquin M Lopez Munoz
+// Distributed under the Boost Software License, Version 1.0.
+// https://www.boost.org/LICENSE_1_0.txt
+
+#include <cstdlib>
+
+#define BOOST_ENABLE_ASSERT_HANDLER 
+
+static bool reentrancy_detected = false;
+
+namespace boost {
+  // Caveat lector: a proper handler shouldn't throw as it may be executed
+  // within a noexcept function.
+
+void assertion_failed_msg(
+  char const*, char const*, char const*, char const*, long)
+{
+  reentrancy_detected = true;
+  throw 0;
+}
+
+void assertion_failed(char const*, char const*, char const*, long) // LCOV_EXCL_START
+{
+  std::abort();
+}                                                                  // LCOV_EXCL_STOP
+
+}
+
+#include <boost/unordered/concurrent_flat_map.hpp>
+#include <boost/core/lightweight_test.hpp>
+
+template<typename F>
+void detect_reentrancy(F f)
+{
+  reentrancy_detected = false;
+  try {
+    f();
+  }
+  catch(int) {}
+  BOOST_TEST(reentrancy_detected);
+}
+
+int main()
+{
+  using map = boost::concurrent_flat_map<int, int>;
+  using value_type = typename map::value_type;
+
+  map m1, m2;
+  m1.emplace(0, 0);
+  m2.emplace(1, 0);
+
+  detect_reentrancy([&] {
+    m1.visit_all([&](value_type&) { (void)m1.contains(0); });
+  }); // LCOV_EXCL_LINE
+
+  detect_reentrancy([&] {
+    m1.visit_all([&](value_type&) { m1.rehash(0); });
+  }); // LCOV_EXCL_LINE
+
+  detect_reentrancy([&] {
+    m1.visit_all([&](value_type&) { 
+      m2.visit_all([&](value_type&) { 
+        m1=m2;
+      }); // LCOV_EXCL_START
+    });
+  });
+  // LCOV_EXCL_STOP
+
+  detect_reentrancy([&] {
+    m1.visit_all([&](value_type&) { 
+      m2.visit_all([&](value_type&) { 
+        m2=m1;
+      }); // LCOV_EXCL_START
+    });
+  });
+  // LCOV_EXCL_STOP
+
+  return boost::report_errors();
+}