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 <[email protected]>
fair-acc · Jul 18, 2024 · a11cf00 · a11cf00
1 parent e608f23
commit a11cf00
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 9 deletions.
diff --git 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
@@ -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";
             }
         };