From 266025c34942f151649e93e51e96419c921b587c Mon Sep 17 00:00:00 2001
From: Alexander Krimm <A.Krimm@gsi.de>
Date: Thu, 18 Jul 2024 14:40:47 +0200
Subject: [PATCH] ClaimStrategy: fix bulk update of AtomicBitset

This fixes the case where setSlotStates is called with (n, n + size).
Before that would lead to no Slots being published, now it should
correctly update the buffer.
This bug can only be reproduced on low-core machines ore by increasing
the number of writers in qa_buffer MultiProducerStdMapMultipleWriters.

fixes #380

Signed-off-by: Alexander Krimm <A.Krimm@gsi.de>
---
 core/include/gnuradio-4.0/ClaimStrategy.hpp |  7 +++----
 core/test/qa_buffer.cpp                     | 10 +++++-----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/core/include/gnuradio-4.0/ClaimStrategy.hpp b/core/include/gnuradio-4.0/ClaimStrategy.hpp
index fd61c6fa..cb638049 100644
--- a/core/include/gnuradio-4.0/ClaimStrategy.hpp
+++ b/core/include/gnuradio-4.0/ClaimStrategy.hpp
@@ -37,7 +37,7 @@ class alignas(hardware_constructive_interference_size) SingleProducerStrategy {
     TWaitStrategy                                           _waitStrategy;
     std::shared_ptr<std::vector<std::shared_ptr<Sequence>>> _readSequences{std::make_shared<std::vector<std::shared_ptr<Sequence>>>()}; // list of dependent reader sequences
 
-    explicit SingleProducerStrategy(const std::size_t bufferSize = SIZE) : _size(bufferSize){};
+    explicit SingleProducerStrategy(const std::size_t bufferSize = SIZE) : _size(bufferSize) {};
     SingleProducerStrategy(const SingleProducerStrategy&)  = delete;
     SingleProducerStrategy(const SingleProducerStrategy&&) = delete;
     void operator=(const SingleProducerStrategy&)          = delete;
@@ -201,13 +201,12 @@ class alignas(hardware_constructive_interference_size) MultiProducerStrategy {
     }
 
     void setSlotsStates(signed_index_type seqBegin, signed_index_type seqEnd, bool value) {
+        assert(seqEnd - seqBegin <= _size && "Begin cannot overturn end");
         const std::size_t beginSet  = static_cast<std::size_t>(seqBegin) & _mask;
         const std::size_t endSet    = static_cast<std::size_t>(seqEnd) & _mask;
         const auto        diffReset = static_cast<std::size_t>(seqEnd - seqBegin);
 
-        if (beginSet == endSet && beginSet == 0UZ && diffReset == _size) {
-            _slotStates.set(0UZ, _size, value);
-        } else if (beginSet <= endSet) {
+        if (beginSet <= endSet && diffReset < _size) {
             _slotStates.set(beginSet, endSet, value);
         } else {
             _slotStates.set(beginSet, _size, value);
diff --git a/core/test/qa_buffer.cpp b/core/test/qa_buffer.cpp
index 70a724da..6d5b30d6 100644
--- a/core/test/qa_buffer.cpp
+++ b/core/test/qa_buffer.cpp
@@ -541,22 +541,22 @@ const boost::ut::suite CircularBufferTests = [] {
                 auto in = reader.get().get();
                 for (const auto& map : in) {
                     auto vIt = map.find(0);
-                    expect(vIt != map.end());
+                    expect(vIt != map.end()) << "map does not contain zero";
                     if (vIt == map.end()) {
                         continue;
                     }
                     const auto value = vIt->second;
-                    expect(ge(value, 0));
-                    expect(le(value, static_cast<int>(kWrites)));
+                    expect(ge(value, 0)) << "value in map should be greater than zero";
+                    expect(le(value, static_cast<int>(kWrites))) << "value in map should be smaller than number of samples to publish";
                     const auto nextIt = std::ranges::find(next, value);
-                    expect(nextIt != next.end());
+                    expect(nextIt != next.end()) << "No writer thread waiting for that number";
                     if (nextIt == next.end()) {
                         continue;
                     }
                     *nextIt = value + 1;
                 }
                 read += in.size();
-                expect(in.consume(in.size()));
+                expect(in.consume(in.size())) << "Failed to consume all";
             }
         };