apollo: bounded_queue Enqueue bug
Describe the bug cyber/base/bounded_queue.h has an bug in its Enqueue method.
template <typename T>
bool BoundedQueue<T>::Enqueue(T&& element) {
uint64_t new_tail = 0;
uint64_t old_commit = 0;
uint64_t old_tail = tail_.load(std::memory_order_acquire);
do {
new_tail = old_tail + 1;
if (GetIndex(new_tail) == GetIndex(head_.load(std::memory_order_acquire))) {
return false;
}
} while (!tail_.compare_exchange_weak(old_tail, new_tail,
std::memory_order_acq_rel,
std::memory_order_relaxed));
pool_[GetIndex(old_tail)] = element;
do {
old_commit = old_tail;
} while (cyber_unlikely(!commit_.compare_exchange_weak(
old_commit, new_tail, std::memory_order_acq_rel,
std::memory_order_relaxed)));
wait_strategy_->NotifyOne();
return true;
}
Line pool_[GetIndex(old_tail)] = element; should be pool_[GetIndex(old_tail)] = std::move(element);
To Reproduce Steps to reproduce the behavior:
- Go to ‘…’
- Click on ‘…’
- Scroll down to ‘…’
- See error
Expected behavior A clear and concise description of what you expected to happen.
Screenshots If applicable, add screenshots to help explain your problem.
Desktop (please complete the following information):
- OS: [e.g. iOS]
- Browser [e.g. chrome, safari]
- Version [e.g. 22]
Smartphone (please complete the following information):
- Device: [e.g. iPhone6]
- OS: [e.g. iOS8.1]
- Browser [e.g. stock browser, safari]
- Version [e.g. 22]
Additional context Add any other context about the problem here.
About this issue
- Original URL
- State: open
- Created 4 years ago
- Reactions: 1
- Comments: 16 (11 by maintainers)
Yes, you‘re right !
elementinTbool BoundedQueue<T>::Enqueue(T&& element)is a lvalue, then we should use std::move to make it a rvalue.I have read the discussion and realized what you mean. What I mean is that, inside a function with prototype func(T&& t), using
std::forward<T>(t)seems a better choice thanstd::move(t).