Merge pull request #165 from laurynas-biveinis/qsbr-deferred_requests…

…-assert-fix Fix race condition in QSBR deallocation request assert
laurynas-biveinis · Mar 23, 2021 · 91c87d8 · 91c87d8
2 parents 83790a7 + ff1e553
commit 91c87d8
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 24 deletions.
diff --git a/qsbr.cpp b/qsbr.cpp
@@ -66,6 +66,7 @@ void qsbr::unregister_thread(std::thread::id thread_id) {
     }
     threads.erase(thread_state);
     --reserved_thread_capacity;
+    requests_to_deallocate.update_single_thread_mode();
 
     if (unlikely(threads.empty())) {
       threads_in_previous_epoch = 0;
@@ -180,22 +181,14 @@ qsbr::deferred_requests qsbr::quiescent_state_locked(
   ++thread_state->second;
   if (thread_state->second > 1) {
     assert_invariants();
-    return deferred_requests{
-#ifndef NDEBUG
-        get_current_epoch()
-#endif
-    };
+    return make_deferred_requests();
   }
 
   --threads_in_previous_epoch;
 
-  deferred_requests result = (threads_in_previous_epoch == 0)
-                                 ? change_epoch()
-                                 : deferred_requests{
-#ifndef NDEBUG
-                                       get_current_epoch()
-#endif
-                                   };
+  deferred_requests result{(threads_in_previous_epoch == 0)
+                               ? change_epoch()
+                               : make_deferred_requests()};
 
   assert_invariants();
 
@@ -209,11 +202,7 @@ qsbr::deferred_requests qsbr::change_epoch() noexcept {
       current_interval_total_dealloc_size.load(std::memory_order_relaxed));
   epoch_callback_stats(previous_interval_deallocation_requests.size());
 
-  deferred_requests result{
-#ifndef NDEBUG
-      get_current_epoch()
-#endif
-  };
+  deferred_requests result{make_deferred_requests()};
   result.requests[0] = std::move(previous_interval_deallocation_requests);
 
   if (likely(!single_thread_mode_locked())) {

diff --git a/qsbr.hpp b/qsbr.hpp
@@ -214,13 +214,13 @@ class qsbr final {
 
     void deallocate(
 #ifndef NDEBUG
-        std::uint64_t dealloc_epoch
+        std::uint64_t dealloc_epoch, bool dealloc_epoch_single_thread_mode
 #endif
     ) const {
       // TODO(laurynas): count deallocation request instances, assert 0 in QSBR
       // dtor
       assert(dealloc_epoch == request_epoch + 2 ||
-             (qsbr::instance().single_thread_mode() &&
+             (dealloc_epoch_single_thread_mode &&
               dealloc_epoch == request_epoch + 1));
 
       detail::pmr_deallocate(pool, pointer, size, alignment);
@@ -234,10 +234,10 @@ class qsbr final {
     deferred_requests() noexcept = default;
 
 #ifndef NDEBUG
-    explicit deferred_requests(std::uint64_t request_epoch_) noexcept
-        : dealloc_epoch{request_epoch_}
-
-    {}
+    deferred_requests(std::uint64_t request_epoch_,
+                      bool dealloc_epoch_single_thread_mode_) noexcept
+        : dealloc_epoch{request_epoch_},
+          dealloc_epoch_single_thread_mode{dealloc_epoch_single_thread_mode_} {}
 #endif
 
     deferred_requests(const deferred_requests &) noexcept = default;
@@ -250,16 +250,25 @@ class qsbr final {
         for (const auto &dealloc_request : reqs) {
           dealloc_request.deallocate(
 #ifndef NDEBUG
-              dealloc_epoch
+              dealloc_epoch, dealloc_epoch_single_thread_mode
 #endif
           );
         }
       }
     }
 
+    // TODO(laurynas): get rid of this wart
+    void update_single_thread_mode() noexcept {
+#ifndef NDEBUG
+      dealloc_epoch_single_thread_mode =
+          qsbr::instance().single_thread_mode_locked();
+#endif
+    }
+
    private:
 #ifndef NDEBUG
     std::uint64_t dealloc_epoch;
+    bool dealloc_epoch_single_thread_mode;
 #endif
   };
 
@@ -287,6 +296,14 @@ class qsbr final {
 
   void assert_invariants() const noexcept;
 
+  deferred_requests make_deferred_requests() noexcept {
+    return deferred_requests{
+#ifndef NDEBUG
+        get_current_epoch(), single_thread_mode_locked()
+#endif
+    };
+  }
+
   std::vector<deallocation_request> previous_interval_deallocation_requests;
   std::vector<deallocation_request> current_interval_deallocation_requests;