CODEWITHSUNDEEP
c++multithreading
Pete Fordham
Pete Fordham

Reputation: 2343

Is this code thread safe? and if not when and how can it go wrong?

OK so I've been told that this code isn't thread safe. 

#include <iostream>
#include <thread>

static int sum[5];

static int get_sum()
{
    int x=0;
    for (int j=0;j<5;++j)
        x += sum[j];
    return x;
}

static void f1(int x){
    sum[x] = 1;
    std::cout << "f" <<x << ": " << sum[x] << " : " << get_sum() << std::endl;
}

int main()  {

    for (int j=0;j<5;++j)
        sum[j] = 0;

    std::thread t0(f1, 0);
    std::thread t1(f1, 1);
    std::thread t2(f1, 2);
    std::thread t3(f1, 3);
    std::thread t4(f1, 4);

    while (get_sum() != 5)  ;

    t0.join();
    t1.join();
    t2.join();
    t3.join();
    t4.join();

    std::cout << "final: " << get_sum() << std::endl;
}

Can someone explain to me why the program might fail to complete? I know the running values of get_sum will be non-deterministic and the output from cout will be randomly interleaved but that's not relevant to the program completing.

Upvotes: 3

Views: 206

Answers (4)

Alvaro Sola
Alvaro Sola

Reputation: 17

You have a problem with the array sum

Upvotes: 0

David Schwartz
David Schwartz

Reputation: 182761

A value may not be accessed in one thread while it might be modified in another. That's the rules.

We can speculate about specific problems that might occur on specific platforms. But generally that's unhelpful because it leads people to think that if they can just fix every possible way they can think of that it might go wrong then their code is okay. But it's not -- things can, and do, go wrong in ways you couldn't have thought of at the time. Please don't start thinking this way. The list of failures caused by this type of reasoning is long and doesn't need to get any longer.

Here's one way it can fail: The compiler might inline get_sum and copy all the values into registers before the while loop, causing the loop to repeat forever. This is a legal optimization because the compiler can see that your code could read any of those values at any time during the while loop, thus no other thread is permitted to modify them during that loop.

Now, if you read this question and the answers, you'll see that a lot of people couldn't think of this way the code can go wrong. But that's okay, they don't have to. These rules exist because it's difficult, perhaps even impossible, to think of all the possible ways code might go wrong. So don't make your code's correctness rely on you having this super-human ability.

Upvotes: 12

Kerrek SB
Kerrek SB

Reputation: 477040

The program is ill-formed because it has undefined behaviour because there is a data race in the access to every single array element, each of which is accessed concurrently by f1 and getsum without any synchronisation.

Upvotes: 1

TieDad
TieDad

Reputation: 9889

This is not safe. Because multiple threads are accessing the global sum. get_sum() might read sum[x] uncertain before or after other threads write to relative position.

To be multithreaed safe, shared resource should be protected by lock, such as mutex.

Upvotes: 0

Related Questions