Filling std::vector by different threads

Question

I need fill one std::vector on differents threads.

Is it correct code? Or I should to add mutex for my code?

void func(int i, std::vector<float>& vec)
{
    vec[i] = i;
}

int main()
{
    std::vector<float> vec(6);
    std::list<std::thread> threads;
    for (int i = 0; i < 6; i++)
    {
        threads.push_back(std::thread(func, i, std::ref(vec)));
    }
    for (auto iter = threads.begin(); iter != threads.end(); iter++)
    {
      (*iter).join();
    }
}

I tested my code it works fine. Are there any pitfalls? Is it thread safe code?

What about get std::vector data by different threads?

Why do you multiply `i` by 4? You are reading your vector out of bounds starting with the third thread. — alter_igel, Apr 03 '20 at 13:52

JohnFilleau · Accepted Answer · 2020-04-03T14:48:31.453

7

Related question:
Is std::vector thread-safe and concurrent by default? Why or why not?.

It's thread-safe because you're not modifying the vector size, and not attempting to write to the same memory location in different threads.

To future proof this answer for anyone who doesn't drill down into the link:

It's not thread safe only because they're using the [] operator. It's thread safe because each thread is explicitly modifying a different location in memory.
If all threads were only reading the same location using [], that would be thread safe.
If all threads were writing to the same same location, using [] won't stop them from messing with each other.
I think if this were production code, at LEAST a comment describing why this is thread safe would be called for. Not sure of any compile time way to prevent someone from shooting themselves in the foot if they modify this function.

On point #4, we want to communicate to future users of this code that:

No we're not guarding this standard library container even though that should be your gut reaction, and
Yes we've analyzed it and it's safe.

The easy way is to stick a comment in there, but there's a saying:

The compiler doesn't read comments and neither do I.
-Bjarne Stroustrup

I think some kind of [[attributes]] should be the way to do this? Although the built-ins don't seem to support any kind of thread safety checking.

Clang appears to provide Thread Safety Analysis:

The analysis is still under active development, but it is mature enough to be deployed in an industrial setting.

Assuming you implement other functions that would require a std::mutex to be responsible for your std::vector:

std::mutex _mu;
std::vector<int> _vec GUARDED_BY(_mu);

then you can explicitly add the NO_THREAD_SAFETY_ANALYSIS attribute to turn off the safety checking for this one specific function. I think it's best to combine this with a comment:

// I know this doesn't look safe but it is as long as
// the caller always launches it with different values of `i`
void foo(int i, std::vector<int>& vec) NO_THREAD_SAFETY_ANALYSIS;

The use of GUARDED_BY tells me, in the future, that you are thinking about thread safety. The use of NO_THREAD_SAFETY_ANALYSIS shows me that you have determined this function is okay to use - especially when other functions that modify your vector are not marked NO_THREAD_SAFETY_ANALYSIS.

edited Apr 03 '20 at 14:48

answered Apr 03 '20 at 13:43

JohnFilleau

4,045
1
15
22

Interesting answer (and question). My instinct would have been that even the `op[]` were not guaranteed safe (even though in practice there's little reason for it to fail). Nice! – Asteroids With Wings Apr 03 '20 at 13:52
what do you mean "out of bounds." is it i pass argument i * 4? – dzukp Apr 03 '20 at 13:52
@dzukp Yes. You have 6 slots but you try to write up to slot 24. You may have intended something like `vec[i/4] = i;`? – Asteroids With Wings Apr 03 '20 at 13:52
Ah, that's better then. – Asteroids With Wings Apr 03 '20 at 13:55
Good shout on the comment, as well. Far too many don't bother with this. – Asteroids With Wings Apr 03 '20 at 13:55
@AsteroidsWithWings can we think of another way to ensure some compile-time warning or error against different threads writing to the same location? I cannot. Comments drift over time. Better than nothing though. Not sure if there's some suite of implementation-defined `[[attributes]]`? – JohnFilleau Apr 03 '20 at 14:09
A bit more formally on the use of `operator []` here: if two or more threads access the same location and at least one of those those threads writes to it, you have a data race, which means you have undefined behavior. That's essentially the wording from the standard, and it's a more dense way of saying what the points numbered 1-3 in the answer say. +1. – Pete Becker Apr 03 '20 at 14:19
fwiw, passing `begin` and `end` iterators instead of the whole container decreases the risk of mistakes, also it makes the range for each thread more explicit – 463035818_is_not_an_ai Apr 03 '20 at 14:19
@JohnFilleau I think that's largely intractible with current technology. There's a reason it's almost never auto-diagnosed – Asteroids With Wings Apr 03 '20 at 14:20
misread the question. I thought every thread fills a range of values, if it is just a single element then pass only a single iterator. Then each thread can only do something obviously wrong but not something that is hard to spot – 463035818_is_not_an_ai Apr 03 '20 at 14:28

score 2 · Answer 2 · answered Apr 03 '20 at 15:00

Yes It's thread safe because you are neither writing to the vector object itself from different threads nor writing to the same object in the vector underlying array .

when you construct the vector you allocate space for 6 elements and fill them with zero for pod types like int. These elements are placed in an array owned and managed by the vector and the vector exposes them via iterators and operator [].

So when you edit an element in the vector you don't edit the vector itself so you don't have to protect the vector with a mutex.

You will need a mutex if you are modifying the vector itself or the same element in different threads .

Filling std::vector by different threads

2 Answers2