From ff1e5538a7a72ba8841ee25e4ba9270d87e30541 Mon Sep 17 00:00:00 2001 From: Laurynas Biveinis Date: Tue, 23 Mar 2021 07:03:26 +0200 Subject: [PATCH] Fix race condition in QSBR deallocation request assert Do not check single thread mode at the deallocation time (as it may have changed by now), but save it at the epoch change time. --- qsbr.cpp | 23 ++++++----------------- qsbr.hpp | 31 ++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/qsbr.cpp b/qsbr.cpp index 70e20d63..9315aeeb 100644 --- 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 index 48af56d8..6da3ae43 100644 --- 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 previous_interval_deallocation_requests; std::vector current_interval_deallocation_requests;