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.

@DanielLangr The sleep_for is just for limiting text output. The real application will be a blocking socket read (with timeout). — rwalton256
– rwalton256, Commented Nov 25, 2020 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<bool> 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
– Daniel Langr, Commented Nov 25, 2020 at 14:58
@DanielLangr I need some sort of locking prevent a setsockopt call from occuring during a read — rwalton256
– rwalton256, Commented Nov 25, 2020 at 15:02

rreignier · Accepted Answer · 2020-11-25 14:55:37Z

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 ) ); } }

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 Perhaps you should make your question a little more specifc. Just edit it to add more information.

Hatted Rooster · Accepted Answer · 2020-11-25 17:27:04Z

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 · Accepted Answer · 2020-11-25 17:35:50Z

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...

Collectives™ on Stack Overflow

STL Mutex not behaving as desired

3 Answers 3

2 Comments

1 Comment

Comments

Linked

Hot Network Questions