Question solved: see the edit.
x86_64, Linux, gcc 11.1, gnu++20.
Before using const std::lock_guard <mutex> lock( mut );
I wanted to give a go to my own implementation of multithreading lock, to have a better understanding of the whole thing.
The problem:
While using the lock_guard is always reliable, that code below isn't: Godbolt snippet
Here is the code, for those willing to read it here:
#include <atomic>
#include <future>
#include <iostream>
#include <mutex>
#include <string>
#include <thread>
#include <vector>
using namespace std;
class Lock_free_sync {
atomic<bool> &object;
const bool expected_input;
bool already_released = false;
bool got_lock = false;
const unsigned debug_id = 0;
public:
Lock_free_sync(atomic<bool> &object_to_lock, const bool expected_input_)
: object(object_to_lock), expected_input(expected_input_) {
get_lock();
}
auto get_lock() -> void{
bool expected_tmp= expected_input;
const bool new_object_value = !expected_tmp;
while (object.compare_exchange_strong(expected_tmp, new_object_value) ==
false) {
expected_tmp= expected_input;
object.wait(expected_tmp);
}
got_lock = true;
}
auto release_lock() -> void{
if (!got_lock) {
return;
}
object = expected_input;
object.notify_all();
already_released = true;
}
~Lock_free_sync() {
if (!already_released) {
release_lock();
}
}
};
mutex h;
auto printer(unsigned id, string &&what) {
#if 0
const std::lock_guard <mutex> lock( h );
cout << id << " " << what << endl;
#endif
}
auto printer(unsigned id, string &&what, unsigned complement) {
#if 0
const std::lock_guard <mutex> lock( h );
cout << id << " " << what << " " << complement << endl;
#endif
}
auto printer2(unsigned id, string &&what, unsigned complement) {
#if 1
// const std::lock_guard <mutex> lock( h );
cout << id << " " << what << " " << complement << endl;
#endif
}
int main() {
atomic<bool> flag = false;
atomic<bool> terminate = false;
auto lambda_thread = [](unsigned i, auto flag_in, auto terminate_in) {
atomic<bool> &flag = flag_in.get();
auto &terminate = terminate_in.get();
printer(i, " >>> in thread");
printer(i, " >>> the bool is lock free? ", flag.is_lock_free());
printer(i, " >>> the bool is always lock free? ",
flag.is_always_lock_free);
unsigned count = 0;
while (1) {
bool expected_value = false;
Lock_free_sync lock(flag, expected_value);
// const std::lock_guard <mutex> lock( h );
printer(i, " >>> in loop working <<<<<<<<<<<<");
++count;
printer(i, " >>> flag is", flag);
this_thread::sleep_for(1s);
printer(i, " >>> check terminate");
// just make sure we are not trapped
if (terminate) {
// const std::lock_guard <mutex> lock( h );
printer2(i, "stop thread", count);
return;
}
printer(i, "about to reloop");
}
};
vector<thread> vect_threads;
{
thread a(lambda_thread, 1, ref(flag), ref(terminate));
vect_threads.push_back(move(a));
}
{
thread a(lambda_thread, 2, ref(flag), ref(terminate));
vect_threads.push_back(move(a));
}
flag.notify_all();
cout << "real sleep" << endl;
this_thread::sleep_for(2s);
cout << "about to call terminate" << endl;
terminate = true;
cout << "wait for join" << endl;
cout << "terminate is" << terminate << endl;
terminate.notify_all();
for (auto &&elem : vect_threads) {
elem.join();
}
cout << "end the main thread" << endl;
return 0;
}
If it shows a result, just reload the page and see it fail (tells time is exceeded because one thread is trapped)
What do I get:
Running this flaky code multiple times will yield correct behaviour sometimes. But at some point, one thread won't finish. The deadlock is here:
bool search_for = locked_value;
const bool new_object_value = !search_for;
while ( object.compare_exchange_strong( search_for, new_object_value ) == false ){
// at this point, search_for has a wrong value in it, otherwise we would not have entered the loop. Set it properly and wait on the value we expect:
search_for = locked_value;
object.wait( search_for );
// at this point, the object has our value but we need to get the flag otherwise, another thread could run the loop at the same time
}
is not doing what I expect it to do.
What I think the above snippet does:
When the program starts, flag is false and so threads will fight to get the lock.
They all compare and exchange and if one of them could get the flag, that one will keep going while the others are waiting in that loop.
Once the work is done, the thread that got the flag will reset it and notify them all, even if it is the last one needing to loop again for it.
Once in the lambda loop, they will read if the terminate flag is set and leave accordingly.
But I know that one thread is trapped in the snippet loop above.
How can I correct it?
#Edit: Code is working now. I had two things wrong:
the scope of the class. It was way too large and bothered the repartition of the resource among threads. Here is the proper version:
while ( 1 ){ //code { // narrow scope Lock_free_sync lock( flag, search_for ); printer( i, " >>> in loop working <<<<<<<<<<<<" ); ++count; printer( i, " >>> flag is", flag ); this_thread::sleep_for( 2ms ); } //code }In the class:
working code in get_lock(), replacing the problematic snippet:
bool search_for = expected_value;
const bool new_object_value = !search_for;
while ( object.compare_exchange_strong( search_for, new_object_value ) == false ){
object.wait( search_for );
// see where search_for is reset? Now it is good.
search_for = expected_value;
}
and in release_lock :
object.exchange( expected_value );
object.notify_all();
Now, after dozens of tries, it works as expected, no more deadlocks.
I learned a lot today. Thanks
