From 7a6564feae07a203742d5609c8f129f05534de99 Mon Sep 17 00:00:00 2001 From: Bartosz Taudul Date: Fri, 30 Aug 2019 14:24:12 +0200 Subject: [PATCH] Only recycle producers, if there's no data in queue. ("The queue" is per-thread partial queue here.) This fixes a problem where one thread writes to the queue, then is terminated, making the (partially filled) queue available for other threads to recycle. If another thread re-owns the queue, it will change the associated thread id, while part of the queue was filled by the original thread. This obviously created invalid data during dequeue. The fix makes the recycling process check not only for queue inactivity (which is marked when the original thread terminates), but also if the queue is empty, preventing mixing data from different threads. --- client/tracy_concurrentqueue.h | 37 ++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/client/tracy_concurrentqueue.h b/client/tracy_concurrentqueue.h index 78178b63..49491622 100644 --- a/client/tracy_concurrentqueue.h +++ b/client/tracy_concurrentqueue.h @@ -2051,23 +2051,26 @@ private: return recycle_or_create_producer(recycled); } - ProducerBase* recycle_or_create_producer(bool& recycled) - { - // Try to re-use one first - for (auto ptr = producerListTail.load(std::memory_order_acquire); ptr != nullptr; ptr = ptr->next_prod()) { - if (ptr->inactive.load(std::memory_order_relaxed)) { - bool expected = true; - if (ptr->inactive.compare_exchange_strong(expected, /* desired */ false, std::memory_order_acquire, std::memory_order_relaxed)) { - // We caught one! It's been marked as activated, the caller can have it - recycled = true; - return ptr; - } - } - } - - recycled = false; - return add_producer(static_cast(create(this))); - } + ProducerBase* recycle_or_create_producer(bool& recycled) + { + // Try to re-use one first + for (auto ptr = producerListTail.load(std::memory_order_acquire); ptr != nullptr; ptr = ptr->next_prod()) { + if (ptr->inactive.load(std::memory_order_relaxed)) { + if( ptr->size_approx() == 0 ) + { + bool expected = true; + if (ptr->inactive.compare_exchange_strong(expected, /* desired */ false, std::memory_order_acquire, std::memory_order_relaxed)) { + // We caught one! It's been marked as activated, the caller can have it + recycled = true; + return ptr; + } + } + } + } + + recycled = false; + return add_producer(static_cast(create(this))); + } ProducerBase* add_producer(ProducerBase* producer) {