patterncppMinor
Multi-thread safe buffer in C++
Viewed 0 times
bufferthreadmultisafe
Problem
I am trying to design thread-safe data structure that I cna use as a buffer in my application. Can you please give me comments about this code and what can be improved:
#include
#include
#include
#include
template
class Buffer
{
public:
void add(T num)
{
while (true)
{
std::unique_lock locker(mu);
buffer.push_back(num);
locker.unlock();
cond.notify_all();
return;
}
}
T remove()
{
while (true)
{
std::unique_lock locker(mu);
cond.wait(locker, [this](){return buffer.size() > 0;});
T back = buffer.back();
buffer.pop_back();
locker.unlock();
cond.notify_all();
return back;
}
}
int size()
{
std::unique_lock locker(mu);
int s = buffer.size();
locker.unlock();
return s;
}
private:
std::mutex mu;
std::condition_variable cond;
std::deque buffer;
};Solution
Problems with
problems with
problems with
add:- the while(true) is useless
- you want to
notify_one, notnotify_allbecause you've only added one thing, and so only one thread waiting to remove can remove it.
- you should overload for two version which take
const T&andT&&(the latter wouldstd::move). This would be optimal in all cases. (analogous topush_backin standard containers)
- You may also want to add an
emplaceto complement theadd()versions so you can constructTin-place at the top of the Buffer.
- you can use
lock_guardinstead ofunique_lockand scope it with{}s. I'd just say always prefer the lighter weight construct when a heavier weight one isn't strictly needed.
problems with
remove:- the
while (true)is useless
- the cond var lambda can be
return !buffer.empty();
- the back variable can be
std::moved
- what happens if you destruct an empty
Bufferwhile a thread is waiting onremove? That case isn't handled at all, and it's a problem
- there's no reason to
notify_allinremove. Who is it notifying and why?
problems with
size:- use a
lock_guardunlessunique_lockis needed
- just do:
std::lock_guard lock{mu}; return buffer.size();
Context
StackExchange Code Review Q#151881, answer score: 6
Revisions (0)
No revisions yet.