Why doesn't mutex work without lock guard?

766 views Asked by At

I have the following code:

#include <chrono>
#include <iostream>
#include <mutex>
#include <thread>

int shared_var {0};
std::mutex shared_mutex;

void task_1()
{
    while (true)
    {
        shared_mutex.lock();
        const auto temp = shared_var;
        std::this_thread::sleep_for(std::chrono::seconds(1));
        
        if(temp == shared_var)
        {
            //do something
        }
        else
        {
            const auto timenow = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());
            std::cout << ctime(&timenow) << ": Data race at task_1: shared resource corrupt \n";
            std::cout << "Actual value: " << shared_var << "Expected value: " << temp << "\n"; 
        }
        shared_mutex.unlock();
    }
}


void task_2()
{
    while (true)
    {
        std::this_thread::sleep_for(std::chrono::seconds(2));
        ++shared_var;
    
    }
}

int main()
{
    auto task_1_thread = std::thread(task_1);
    auto task_2_thread = std::thread(task_2);
 
    task_1_thread.join();
    task_2_thread.join();
    return 0;
}

shared_var is protected in task_1 but not protected in task_2 What is expected: I was expecting else branch is not entered in task_1 as the shared resource is locked. What actually happens: Running this code will enter else branch in task_1.

Expected outcome is obtained when replace shared_mutex.lock(); with std::lock_guard<std::mutex> lock(shared_mutex); and shared_mutex.unlock(); with std::lock_guard<std::mutex> unlock(shared_mutex);

Questions:

  1. What is the problem in my current approach?
  2. Why does it work with loack_guard?

I am running the code on: https://www.onlinegdb.com/online_c++_compiler

3

There are 3 answers

0
463035818_is_not_an_ai On BEST ANSWER

Suppose you have a room with two entries. One entry has a door the other not. The room is called shared_var. There are two guys that want to enter the room, they are called task_1 and task_2.

You now want to make sure somehow that only one of them is inside the room at any time.

taks_2 can enter the room freely through the entry without a door. task_1 uses the door called shared_mutex.

Your question is now: Can achieve that only one guy is in the room by adding a lock to the door at the first entry?

Obviously no, because the second door can still be entered and left without you having any control over it.

If you experiment you might observe that without the lock it happens that you find both guys in the room while after adding the lock you don't find both guys in the room. Though this is pure luck (bad luck actually, because it makes you beleive that the lock helped). In fact the lock did not change much. The guy called task_2 can still enter the room while the other guy is inside.

The solution would be to make both go through the same door. They lock the door when going inside and unlock it when leaving the room. Putting an automatic lock on the door can be nice, because then the guys cannot forget to unlock the door when they leave.

Oh sorry, i got lost in telling a story.

TL;DR: In your code it does not matter if you use the lock or not. Actually also the mutex in your code is useless, because only one thread un/locks it. To use the mutex properly, both threads need to lock it before reading/writing shared memory.

0
Joachim Isaksson On

A mutex lock does not lock a variable, it just locks the mutex so that other code cannot lock the same mutex at the same time.

In other words, all accesses to a shared variable need to be wrapped in a mutex lock on the same mutex to avoid multiple simultaneous accesses to the same variable, it's not in any way automatic just because the variable is wrapped in a mutex lock in another place in the code.

You're not locking the mutex at all in task2, so there is a race condition.

The reason it seems to work when you wrap the mutex in a std::lock_guard is that the lock guard holds the mutex lock until the end of the scope which in this case is the end of the function.

Your function first locks the mutex with the lock lock_guard to later in the same scope try to lock the same mutex with the unlock lock_guard. Since the mutex is already locked by the lock lock_guard, execution stops and there is no output because the program is in effect not running anymore.

If you output "ok" in your code at the point of the "//do something" comment, you'll see that you get the output once and then the program stops all output.

Note; as of this behaviour being guaranteed, see @Jarod42s answer for much better info on that. As with most unexpected behaviour in C++, there is probably an UB involved.

0
Jarod42 On

With UB (as data race), output is undetermined, you might see "expected" output, or strange stuff, crash, ...

  1. What is the problem in my current approach?

In first sample, you have data race as you write (non-atomic) shared_var in one thread without synchronization and read in another thread.

  1. Why does it work with loack_guard?

In modified sample, you lock twice the same (non-recursive) mutex, which is also UB

From std::mutex::lock:

If lock is called by a thread that already owns the mutex, the behavior is undefined

You just have 2 different behaviours for 2 different UB (when anything can happen for both cases).