STL Mutex not behaving as desired

Question

I couldn't find much on this so I thought I'd post my own question.

I have a thread that executes a loop, locking a mutex each time. The issue is, there is not enough time between loops for another thread to lock the mutex. Here is some code that replicates my issue:

#include <thread>
#include <mutex>
#include <iostream>
#include <functional>

volatile bool bar;

void foo( std::mutex& m )
{
    while( true )
    {
        std::unique_lock<std::mutex> lock( m );

        if( bar )
            break;

        std::cout << "Hello World" << std::endl;
 
        std::this_thread::sleep_for( std::chrono::seconds( 1 ) );
    }
}

int main( int argc, char** argv )
{
    bar = false;

    std::mutex m;

    std::thread t( std::bind( foo, std::ref( m ) ) );

    std::this_thread::sleep_for( std::chrono::seconds( 5 ) );

    std::cout << "Terminating thread..." << std::endl;

    {
        std::unique_lock<std::mutex> lock( m );

        bar = true;
    }

    t.join();
}

And sample output:

Hello World
Hello World
Hello World
Hello World
Hello World
Terminating thread...
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World

I'm sure a simple sleep command before taking the mutex will solve my issue, but I'd like to avoid that if necessary due to the high loop rate and low latency requirements of my application.

Any ideas here? Is there a way to notify the main thread? Is std::mutex the best choice here?

Thanks.

Relevant: [Do mutexes guarantee ordering of acquisition?](https://stackoverflow.com/q/37860811/580083). — Daniel Langr, Nov 25 '20 at 14:53
@DanielLangr The sleep_for is just for limiting text output. The real application will be a blocking socket read (with timeout). — rwalton256, Nov 25 '20 at 14:54
IMO, a mutex is not a right tool for your problem. Why don't you use a simple atomic flag? Change the type of `bar` to `std::atomic` and don't lock the mutex in the main thread at all. That is, protect the socket use with the mutex, but not the notification flat. — Daniel Langr, Nov 25 '20 at 14:58
@DanielLangr I need some sort of locking prevent a `setsockopt` call from occuring during a `read` — rwalton256, Nov 25 '20 at 15:02
Unrelated: rather than `std::bind` and `std::ref`, see if you can use a lambda. Usually a bit cleaner. — user4581301, Nov 25 '20 at 15:03
@rwalton256 Yes, protect the socket manipulation, but not the flag (`bar` here). — Daniel Langr, Nov 25 '20 at 15:04
@DanielLangr Sure, I see what you mean now. That's a good idea thank you. — rwalton256, Nov 25 '20 at 15:07
`volatile` doesn’t do anything useful here. Make `bar` `std::atomic`. But the code still won’t have any guarantee that `main` will get the lock. — Pete Becker, Nov 25 '20 at 15:43
@MichaëlRoy Blocking sockets with a timeout work well for my application. — rwalton256, Nov 25 '20 at 17:40
@rwalton256 Sure.... except when you need to do that pesky call to setsockopt().... — Michaël Roy, Nov 25 '20 at 21:57

score 1 · Answer 1 · answered Nov 25 '20 at 14:55

1

You must add a scope around your lock otherwise the mutex is still locked while sleeping:

void foo( std::mutex& m )
{
    while( true )
    {
        {
            std::unique_lock<std::mutex> lock( m );
            if( bar )
                break;
        }
        // The mutex is now unlocked

        std::cout << "Hello World" << std::endl;
 
        std::this_thread::sleep_for( std::chrono::seconds( 1 ) );
    }
}

answered Nov 25 '20 at 14:55

rreignier

31
2

This doesn't solve my problem, the sleep_for and print statements are just examples. My actual application requires the mutex to be locked. – rwalton256 Nov 25 '20 at 14:57
@rwalton256 Perhaps you should make your question a little more specifc. Just edit it to add more information. – Ted Lyngmo Jan 03 '21 at 23:59

rwalton256 · Answer 2 · 2020-11-25T17:35:50.297

I've found a fix that works well enough for me:

#include <thread>
#include <mutex>
#include <iostream>
#include <functional>
#include <atomic>
#include <condition_variable>

std::atomic<bool> bar, pause;

void foo( std::mutex& m, std::condition_variable& v, std::string& s )
{
    std::unique_lock<std::mutex> lock( m );
    while( bar.load() )
    {
        v.wait( lock, [=] { return !pause.load(); } );

        std::cout << s << std::endl;
 
        std::this_thread::sleep_for( std::chrono::seconds( 1 ) );
    }
}

int main( int argc, char** argv )
{
    bar.store( true );
    pause.store( false );

    std::mutex m;
    std::condition_variable v;
    std::string s("Hello World");

    std::thread t( std::bind( foo, std::ref( m ), std::ref( v ), std::ref( s ) ) );

    std::this_thread::sleep_for( std::chrono::seconds( 5 ) );

    std::cout << "Modifying string..." << std::endl;

    {
        pause.store( true );

        std::unique_lock<std::mutex> lock( m );

        s = std::string("Goodbye World");

        pause.store( false );
        v.notify_one();
    }

    std::this_thread::sleep_for( std::chrono::seconds( 5 ) );

    std::cout << "Terminating thread..." << std::endl;

    bar.store( false );

    t.join();
}

Output:

Hello World
Hello World
Hello World
Hello World
Hello World
Modifying string...
Goodbye World
Goodbye World
Goodbye World
Goodbye World
Goodbye World
Terminating thread...

score 0 · Answer 3 · answered Nov 25 '20 at 17:27

Your design is flawed. Your other answer already answers how to solve this but I'll answer why it happens.

A common optimization technique for std::mutex is to just hold the lock of the current executing thread if it will be unlocked and immediately locked again. Here:

while( true )
{
    std::unique_lock<std::mutex> lock( m ); // lock taken

    if( bar )
        break;

    std::cout << "Hello World" << std::endl;
 
    std::this_thread::sleep_for( std::chrono::seconds( 1 ) );
    //lock destroyed, m unlocked.
}

The moment that the sleep is over lock is destroyed and m is unlocked but not really because the loop body starts immediately from the start again and will relock m. This results in a permanent lock on that thread and starvation on the main thread.

Yes I know what the issue is... I was looking for solutions. And also, it wasn't permanent starvation. I ran the first example and after a few minutes it did return. — rwalton256, Nov 25 '20 at 17:33

STL Mutex not behaving as desired

3 Answers3