`sync_bounded_queue::size` may return an incorrect value

Open
#414 0 comments 8 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
58/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Stale
Tech stack
cpp

Research direction

Start with include/boost/thread/concurrent_queues/sync_bounded_queue.hpp and inspect the size implementation alongside full(), capacity(), and the in_/out_ state. Reproduce the wraparound sequence from the issue, then add or adapt a regression test covering push and pull cycles; done means size() matches the number of queued elements after wraparound.

Written by the indexing model from the issue text.

Description

Bug Report: sync_bounded_queue::size may return an incorrect value

Description

The sync_bounded_queue::size method can return an incorrect value under specific conditions.

Steps to Reproduce
  1. Create a sync_bounded_queue with a fixed capacity (e.g., 5).
  2. Fill the queue to its maximum capacity using try_push.
  3. Empty the queue completely using try_pull.
  4. Push a single element into the queue.

After these steps, calling size() returns 0 instead of the expected 1. However, the empty() method correctly returns false.

Minimal Reproducible Example
#include <iostream>
#include <boost/thread/concurrent_queues/sync_bounded_queue.hpp>
int main() {
    const size_t capacity = 5;
    boost::sync_bounded_queue<int> queue(capacity);
    size_t test_size = 0;
    for (size_t i = 0; i < capacity; ++i) {
        if (queue.try_push(i)) {
            ++test_size;
        }
    }
    for (size_t i = 0; i < capacity; ++i) {
        if (queue.try_pull()) {
            --test_size;
        }
    }
    if (queue.try_push(1)) {
        ++test_size;
    }
    std::cout << "size = " << queue.size() << "\ttest_size = " << test_size << std::endl;
    std::cout << "empty = " << queue.empty() << "\ttest_empty = " << (test_size == 0) << std::endl;
    std::cout << "[ TEST " << (queue.size() == test_size ? "PASSED" : "FAILED") << "]" << std::endl;
    return 0;
}

Live Demo: https://godbolt.org/z/hMTvTT6Kq

Actual Output
size = 0	test_size = 1
empty = 0	test_empty = 0
[ TEST FAILED]
Expected Output
size = 1	test_size = 1
empty = 0	test_empty = 0
[ TEST PASSED]
Proposed Fix

The issue seems to be in the size method implementation. The current logic incorrectly handles the calculation when the queue has been wrapped around. The proposed fix simplifies the calculation:

--- a/include/boost/thread/concurrent_queues/sync_bounded_queue.hpp
+++ b/include/boost/thread/concurrent_queues/sync_bounded_queue.hpp
@@ -126,8 +126,7 @@ namespace concurrent
     }
     inline size_type size(lock_guard<mutex>& lk) const BOOST_NOEXCEPT
     {
-      if (full(lk)) return capacity(lk);
-      return ((in_+capacity(lk)-out_) % capacity(lk));
+      return ((in_+capacity_-out_) % capacity_);
     }
Validation

A comprehensive test has been created to verify the fix works correctly across multiple operations:

#include <iostream>
#include <boost/thread/concurrent_queues/sync_bounded_queue.hpp>

bool test_queue(boost::sync_bounded_queue<int>& queue, size_t& test_size, size_t count) {
    for (size_t i = 0; i < 2 * queue.capacity(); ++i) {
        for (size_t j = 0; j < count; ++j) {
            ++test_size;
            if (!queue.try_push(0) || queue.size() != test_size) {
                std::cout << "[ TEST FAILED ]" << std::endl;
                return false;
            }
        }
        for (size_t j = 0; j < count; ++j) {
            --test_size;
            if (!queue.try_pull() || queue.size() != test_size) {
                std::cout << "[ TEST FAILED ]" << std::endl;
                return false;
            }
        }
    }
    return true;
}

int main() {
    const size_t capacity = 5;
    boost::sync_bounded_queue<int> queue(capacity);
    size_t test_size = 0;
    for (size_t i = 1; i <= capacity; ++i) {
        if (!test_queue(queue, test_size, i)) {
            return 1;
        }
    }
    return 0;
}

Test with current implementation (fails): https://godbolt.org/z/48nTe4zYP
Test with proposed fix (passes): https://godbolt.org/z/33Ysr9hsE

Environment
  • Compiler: (any, reproduced on Godbolt with various compilers)
  • Platform: (any)

sync_bounded_queue.patch

Dominant language
C++
Stars
213
Forks
171
PR merge metrics
No merged PRs in 30d

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from boostorg/thread

All issues in boostorg/thread

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.