I have multiple threads modifying an stl vector and an stl list.
I want to avoid having to take a lock if the container is empty
Would the following code be threadsafe? What if items was a list or a map?
class A
{
vector<int> items
void DoStuff()
{
if(!items.empty())
{
AquireLock();
DoStuffWithItems();
ReleaseLock();
}
}
}
It depends what you expect. The other answers are right that in general, standard C++ containers are not thread-safe, and furthermore, that in particular your code doesn’t ward against another thread modifying the container between your call to empty and the acquisition of the lock (but this matter is unrelated to the thread safety of vector::empty).
So, to ward off any misunderstandings: Your code does not guarantee items will be non-empty inside the block.
But your code can still be useful, since all you want to do is avoid redundant lock creations. Your code doesn’t give guarantees but it may prevent an unnecessary lock creation. It won’t work in all cases (other threads can still empty the container between your check and the lock) but in some cases. And if all you’re after is an optimization by omitting a redundant lock, then your code accomplishes that goal.
Just make sure that any actual access to the container is protected by locks.
By the way, the above is strictly speaking undefined behaviour: an STL implementation is theoretically allowed to modify mutable members inside the call to empty. This would mean that the apparently harmless (because read-only) call to empty can actually cause a conflict. Unfortunately, you cannot rely on the assumption that read-only calls are safe with STL containers.
In practice, though, I am pretty sure that vector::empty will not modify any members. But already for list::empty I am less sure. If you really want guarantees, then either lock every access or don’t use the STL containers.
There is no thread-safe guaranty on anything in the containers and algorithms of the the STL.
So, No.
As it is already answered, the above code is not thread safe and locking is mandatory before actually doing anything with the container. But the following should have better performance than always locking and I can't think of a reason that it can be unsafe. The idea here is that locking can be expensive and we are avoiding it, whenever not really needed.
class A
{
vector<int> items;
void DoStuff()
{
if(!items.empty())
{
AquireLock();
if(!items.empty())
{
DoStuffWithItems();
}
ReleaseLock();
}
}
}
Regardless of whether or not empty is thread safe, your code will not, as written, accomplish your goal.
class A
{
vector<int> items
void DoStuff()
{
if(!items.empty())
{
//Another thread deletes items here.
AquireLock();
DoStuffWithItems();
ReleaseLock();
}
}
}
A better solution is to lock every time you work with items (when iterating, getting items, adding items, checking count/emptiness, etc.), thus providing your own thread safety. So, acquire the lock first, then check if the vector is empty.
STL is not thread safe and empty too. If you want make container safe you must close all its methods by mutex or other sync
