Why the following program does not mix the output when mutex is not used?

Question

I have made multiple runs of the program. I do not see that the output is incorrect, even though I do not use the mutex. My goal is to demonstrate the need of a mutex. My thinking is that different threads with different "num" values will be mixed.

Is it because the objects are different?

using VecI = std::vector<int>;
class UseMutexInClassMethod {
    mutex m;
public:
    VecI compute(int num, VecI veci)
    {
        VecI v;
        num = 2 * num -1;
        for (auto &x:veci) {
            v.emplace_back(pow(x,num));
            std::this_thread::sleep_for(std::chrono::seconds(1));

        }
        return v;
    }

};  

void TestUseMutexInClassMethodUsingAsync()
{
    const int nthreads = 5;
    UseMutexInClassMethod useMutexInClassMethod;
    VecI vec{ 1,2,3,4,5 };
    std::vector<std::future<VecI>> futures(nthreads);
    std::vector<VecI> outputs(nthreads);

    for (decltype(futures)::size_type i = 0; i < nthreads; ++i) {
        futures[i] = std::async(&UseMutexInClassMethod::compute,
            &useMutexInClassMethod,
            i,vec
        );
    }

    for (decltype(futures)::size_type i = 0; i < nthreads; ++i) {
        outputs[i] = futures[i].get();
        for (auto& x : outputs[i])
            cout << x << " ";
        cout << endl;
    }

}

Answer 1

If you want an example that does fail with a high degree of certainty you can look at the below. It sets up a variable called accumulator to be shared by reference to all the futures. This is what is missing in your example. You are not actually sharing any memory. Make sure you understand the difference between passing by reference and passing by value.

#include <vector>
#include <memory>
#include <thread>
#include <future>
#include <iostream>
#include <cmath>
#include <mutex>


struct UseMutex{
    int compute(std::mutex & m, int & num)
    {
        for(size_t j = 0;j<1000;j++)
        {
            ///////////////////////
            // CRITICAL SECTIION //
            ///////////////////////

            // this code currently doesn't trigger the exception
            // because of the lock on the mutex. If you comment
            // out the single line below then the exception *may*
            // get called.
            std::scoped_lock lock{m};

            num++;
            std::this_thread::sleep_for(std::chrono::nanoseconds(1));
            num++;
            if(num%2!=0)
                throw std::runtime_error("bad things happened");
        }

        return 0;
    }
};  

template <typename T> struct F;

void TestUseMutexInClassMethodUsingAsync()
{
  
    const int nthreads = 16;
    int accumulator=0;
    std::mutex m;
    std::vector<UseMutex> vs{nthreads};
    std::vector<std::future<int>> futures(nthreads);


    for (auto i = 0; i < nthreads; ++i) {
        futures[i]= std::async([&,i](){return vs[i].compute(m,accumulator);});
    }

    for(auto i = 0; i < nthreads; ++i){
        futures[i].get(); 
    }


}

int main(){
    TestUseMutexInClassMethodUsingAsync();
}

You can comment / uncomment the line

std::scoped_lock lock{m};

which protects the increment of the shared variable num. The rule for this mini program is that at the line

 if(num%2!=0)
                throw std::runtime_error("bad things happened");

num should be a multiple of two. But as multiple threads are accessing this variable without a lock you can't guarantee this. However if you add a lock around the double increment and test then you can be sure no other thread is accessing this memory during the duration of the increment and test.

Failing https://godbolt.org/z/sojcs1WK9

Passing https://godbolt.org/z/sGdx3x3q3

Of course the failing one is not guaranteed to fail but I've set it up so that it has a high probability of failing.

Notes

[&,i](){return vs[i].compute(m,accumulator);};

is a lambda or inline function. The notation [&,i] means it captures everything by reference except i which it captures by value. This is important because i changes on each loop iteration and we want each future to get a unique value of i

Answer 2

Is it because the objects are different?

Yes.

Your code is actually perfectly thread safe, no need for mutex here. You never share any state between threads except for copying vec from TestUseMutexInClassMethodUsingAsync to compute by std::async (and copying is thread-safe) and moving computation result from compute's return value to futures[i].get()'s return value. .get() is also thread-safe: it blocks until the compute() method terminates and then returns its computation result.

It's actually nice to see that even a deliberate attempt to get a race condition failed :)

You probably have to fully redo your example to demonstrate is how simultaneous* access to a shared object breaks things. Get rid of std::async and std::future, use simple std::thread with capture-by-reference, remove sleep_for (so both threads do a lot of operations instead of one per second), significantly increase number of operations and you will get a visible race. It may look like a crash, though.

* - yes, I'm aware that "wall-clock simulateneous access" does not exist in multithreaded systems, strictly speaking. However, it helps getting a rough idea of where to look for visible race conditions for demonstration purposes.