From a0c7112652b2c22fbee3bac62aa81ff1dabc8140 Mon Sep 17 00:00:00 2001
From: Christian Mazakas <christian.mazakas@gmail.com>
Date: Wed, 13 Sep 2023 10:16:20 -0700
Subject: [PATCH] Update arrays transfer in table_core moves constructors to be
 exception-safe

---
 .../unordered/detail/foa/concurrent_table.hpp |  27 +++--
 include/boost/unordered/detail/foa/core.hpp   | 106 +++++++++++++-----
 include/boost/unordered/detail/foa/table.hpp  |  47 ++++----
 3 files changed, 119 insertions(+), 61 deletions(-)

diff --git a/include/boost/unordered/detail/foa/concurrent_table.hpp b/include/boost/unordered/detail/foa/concurrent_table.hpp
index f7881e8e..3a90c2f4 100644
--- a/include/boost/unordered/detail/foa/concurrent_table.hpp
+++ b/include/boost/unordered/detail/foa/concurrent_table.hpp
@@ -488,21 +488,32 @@ public:
   concurrent_table(concurrent_table&& x,const Allocator& al_):
     concurrent_table(std::move(x),al_,x.exclusive_access()){}
 
-  concurrent_table(compatible_nonconcurrent_table&& x):
+  template<typename ArraysType>
+  concurrent_table(
+    compatible_nonconcurrent_table&& x,
+    arrays_holder<ArraysType,Allocator>&& ah):
     super{
       std::move(x.h()),std::move(x.pred()),std::move(x.al()),
-      arrays_type(arrays_type::new_group_access(
-        x.al(),
-        typename arrays_type::super{
+      [&x]{return arrays_type::new_group_access(
+        x.al(),typename arrays_type::super{
           x.arrays.groups_size_index,x.arrays.groups_size_mask,
-          boost::pointer_traits<typename arrays_type::group_type_pointer>::pointer_to(
-            *reinterpret_cast<group_type*>(x.arrays.groups())),
-          x.arrays.elements_})),
+          to_pointer<typename arrays_type::group_type_pointer>(
+            reinterpret_cast<group_type*>(x.arrays.groups())),
+          x.arrays.elements_});},
       size_ctrl_type{x.size_ctrl.ml,x.size_ctrl.size}}
   {
-    x.empty_initialize();
+    ah.release();
+    x.arrays=ah.get();
+    x.size_ctrl.ml=x.initial_max_load();
+    x.size_ctrl.size=0;
   }
 
+  concurrent_table(compatible_nonconcurrent_table&& x):
+    concurrent_table(std::move(x), arrays_holder<
+      typename compatible_nonconcurrent_table::arrays_type,Allocator
+    >{compatible_nonconcurrent_table::arrays_type::new_(x.al(),0),x.al()})
+  {}
+
   ~concurrent_table()=default;
 
   concurrent_table& operator=(const concurrent_table& x)
diff --git a/include/boost/unordered/detail/foa/core.hpp b/include/boost/unordered/detail/foa/core.hpp
index 2b2104d3..c1fb4850 100644
--- a/include/boost/unordered/detail/foa/core.hpp
+++ b/include/boost/unordered/detail/foa/core.hpp
@@ -938,6 +938,55 @@ Group* dummy_groups()
     const_cast<typename Group::dummy_group_type*>(storage));
 }
 
+template<
+  typename Ptr,typename Ptr2,
+  typename std::enable_if<!std::is_same<Ptr,Ptr2>::value>::type* = nullptr
+>
+Ptr to_pointer(Ptr2 p)
+{
+  if(!p){return nullptr;}
+  return boost::pointer_traits<Ptr>::pointer_to(*p);
+}
+
+template<typename Ptr>
+Ptr to_pointer(Ptr p)
+{
+  return p;
+}
+
+template<typename Arrays,typename Allocator>
+struct arrays_holder
+{
+  arrays_holder(Arrays const& arrays, Allocator const& al)
+    :arrays_{arrays},al_{al}
+  {}
+  
+  arrays_holder(arrays_holder const&)=delete;
+  arrays_holder& operator=(arrays_holder const&)=delete;
+
+  ~arrays_holder()
+  {
+    if (!released_){
+      arrays_.delete_(al_,arrays_);
+    }
+  }
+
+  Arrays const& get()const
+  {
+    return arrays_;
+  }
+
+  void release()
+  {
+    released_=true;
+  }
+
+private:
+  Arrays arrays_;
+  Allocator al_;
+  bool released_=false;
+};
+
 template<typename Value,typename Group,typename SizePolicy,typename Allocator>
 struct table_arrays
 {
@@ -1309,20 +1358,34 @@ public:
     size_ctrl{initial_max_load(),0}
     {}
 
-  /* bare transfer ctor for concurrent/non-concurrent interop */
-
+  /* genericize on an ArraysFn so that we can do things like delay an
+   * allocation for the group_access data required by cfoa after the move
+   * constructors of Hash, Pred have been invoked
+   */
+  template<typename ArraysFn>
   table_core(
     Hash&& h_,Pred&& pred_,Allocator&& al_,
-    const arrays_type& arrays_,const size_ctrl_type& size_ctrl_):
+    ArraysFn arrays_fn,const size_ctrl_type& size_ctrl_):
     hash_base{empty_init,std::move(h_)},
     pred_base{empty_init,std::move(pred_)},
     allocator_base{empty_init,std::move(al_)},
-    arrays(arrays_),size_ctrl(size_ctrl_)
+    arrays(arrays_fn()),size_ctrl(size_ctrl_)
   {}
 
   table_core(const table_core& x):
     table_core{x,alloc_traits::select_on_container_copy_construction(x.al())}{}
 
+  template<typename ArraysFn>
+  table_core(table_core&& x,arrays_holder<arrays_type,Allocator>&& ah,ArraysFn arrays_fn):
+    table_core(
+      std::move(x.h()),std::move(x.pred()),std::move(x.al()),arrays_fn,x.size_ctrl)
+  {
+    ah.release();
+    x.arrays=ah.get();
+    x.size_ctrl.ml=x.initial_max_load();
+    x.size_ctrl.size=0;
+  }
+
   table_core(table_core&& x)
     noexcept(
       std::is_nothrow_move_constructible<Hash>::value&&
@@ -1330,11 +1393,9 @@ public:
       std::is_nothrow_move_constructible<Allocator>::value&&
       !uses_fancy_pointers):
     table_core{
-      std::move(x.h()),std::move(x.pred()),std::move(x.al()),
-      x.arrays,x.size_ctrl}
-  {
-    x.empty_initialize();
-  }
+      std::move(x),arrays_holder<arrays_type,Allocator>{x.new_arrays(0),x.al()},
+      [&x]{return x.arrays;}}
+  {}
 
   table_core(const table_core& x,const Allocator& al_):
     table_core{std::size_t(std::ceil(float(x.size())/mlf)),x.h(),x.pred(),al_}
@@ -1372,11 +1433,17 @@ public:
     delete_arrays(arrays);
   }
 
-  void empty_initialize()noexcept(!uses_fancy_pointers)
+  std::size_t initial_max_load()const
   {
-    arrays=new_arrays(0);
-    size_ctrl.ml=initial_max_load();
-    size_ctrl.size=0;
+    static constexpr std::size_t small_capacity=2*N-1;
+
+    auto capacity_=capacity();
+    if(capacity_<=small_capacity){
+      return capacity_; /* we allow 100% usage */
+    }
+    else{
+      return (std::size_t)(mlf*(float)(capacity_));
+    }
   }
 
   table_core& operator=(const table_core& x)
@@ -2022,19 +2089,6 @@ private:
     recover_slot(reinterpret_cast<unsigned char*>(pg)+pos);
   }
 
-  std::size_t initial_max_load()const
-  {
-    static constexpr std::size_t small_capacity=2*N-1;
-
-    auto capacity_=capacity();
-    if(capacity_<=small_capacity){
-      return capacity_; /* we allow 100% usage */
-    }
-    else{
-      return (std::size_t)(mlf*(float)(capacity_));
-    }
-  }  
-
   static std::size_t capacity_for(std::size_t n)
   {
     return size_policy::size(size_index_for<group_type,size_policy>(n))*N-1;
diff --git a/include/boost/unordered/detail/foa/table.hpp b/include/boost/unordered/detail/foa/table.hpp
index 46335568..f8308b1e 100644
--- a/include/boost/unordered/detail/foa/table.hpp
+++ b/include/boost/unordered/detail/foa/table.hpp
@@ -133,22 +133,6 @@ private:
   template<typename> friend class table_erase_return_type;
   template<typename,typename,typename,typename> friend class table;
 
-  template<
-    typename Ptr,typename Ptr2,
-    typename std::enable_if<!std::is_same<Ptr,Ptr2>::value>::type* = nullptr
-  >
-  static Ptr to_pointer(Ptr2 p)
-  {
-    if(!p){return nullptr;}
-    return boost::pointer_traits<Ptr>::pointer_to(*p);
-  }
-
-  template<typename Ptr>
-  static Ptr to_pointer(Ptr p)
-  {
-    return p;
-  }
-
   table_iterator(group_type* pg,std::size_t n,const table_element_type* p):
     pc_{to_pointer<char_pointer>(
       reinterpret_cast<unsigned char*>(const_cast<group_type*>(pg))+n)},
@@ -560,23 +544,32 @@ public:
   friend bool operator!=(const table& x,const table& y){return !(x==y);}
 
 private:
-  template<typename ExclusiveLockGuard>
-  table(compatible_concurrent_table&& x,ExclusiveLockGuard):
+  template<typename ArraysType>
+  table(compatible_concurrent_table&& x,arrays_holder<ArraysType,Allocator>&& ah):
     super{
       std::move(x.h()),std::move(x.pred()),std::move(x.al()),
-      arrays_type{
+      [&x]{return arrays_type{
         x.arrays.groups_size_index,x.arrays.groups_size_mask,
-        boost::pointer_traits<typename arrays_type::group_type_pointer>::pointer_to(
-            *reinterpret_cast<group_type*>(x.arrays.groups())),
-        x.arrays.elements_},
-      size_ctrl_type{
-        x.size_ctrl.ml,x.size_ctrl.size}}
+        to_pointer<group_type_pointer>(
+          reinterpret_cast<group_type*>(x.arrays.groups())),
+        x.arrays.elements_};},
+      size_ctrl_type{x.size_ctrl.ml,x.size_ctrl.size}}
   {
-    compatible_concurrent_table::arrays_type::delete_group_access(
-      this->al(),x.arrays);
-    x.empty_initialize();
+    ah.release();
+
+    compatible_concurrent_table::arrays_type::delete_group_access(x.al(),x.arrays);
+    x.arrays=ah.get();
+    x.size_ctrl.ml=x.initial_max_load();
+    x.size_ctrl.size=0;
   }
 
+  template<typename ExclusiveLockGuard>
+  table(compatible_concurrent_table&& x,ExclusiveLockGuard):
+    table(std::move(x),arrays_holder<
+      typename compatible_concurrent_table::arrays_type,Allocator
+    >{compatible_concurrent_table::arrays_type::new_(x.al(),0),x.al()})
+  {}
+
   struct erase_on_exit
   {
     erase_on_exit(table& x_,const_iterator it_):x{x_},it{it_}{}