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.

Answer 3

Comments have called out the fact that just not protecting a critical section does not guarantee that the risked behavior actually occurs.
That also applies for multiple runs, because while you are not allowed to test a few times and then rely on the repeatedly observed behavior, it is likely that optimization mechanisms cause a likely enough reoccurring observation as to be perceived has reproducible.

If you intend to demonstrate the need for synchronization you need to employ synchronization to poise things to a near guaranteed misbehavior of observable lack of protection.

Allow me to only outline a sequence for that, with a few assumptions on scheduling mechanisms (this is based on a rather simple, single core, priority based scheduling environment I have encountered in an embedded environment I was using professionally), just to give an insight with a simplified example:

• start a lower priority context.
• optionally set up proper protection before entering the critical section
• start critical section, e.g. by outputting the first half of to-be-continuous output
• asynchronously trigger a higher priority context, which is doing that which can violate your critical section, e.g. outputs something which should not be in the middle of the two-part output of the critical section
• (in protected case the other context is not executed, in spite of being higher priority)
• (in unprotected case the other context is now executed, because of being higher priority)
• end critical section, e.g. by outputting the second half of the to-be-continuous output
• optionally remove the protection after leaving the critical section
• (in protected case the other context is now executed, now that it is allowed)
• (in unprotected case the other context was already executed)

Note:
I am using the term "critical section" with the meaning of a piece of code which is vulnerable to being interrupted/preempted/descheduled by another piece of code or another execution of the same code. Specifically for me a critical section can exist without applied protection, though that is not a good thing. I state this explicitly because I am aware of the term being used with the meaning "piece of code inside applied protection/synchronization". I disagree but I accept that the term is used differently and requires clarification in case of potential conflicts.