6
\$\begingroup\$

I am trying to make a thread test in which 4 threads are all trying to set a bool. Only one should be allowed to set it. After the first thread has set the bool (m_isRunning) to true, no other threads should set it to true because of the check isRunning().

This implementation works, but I wonder if this is the best/most efficient approach to this common situation.

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

std::mutex mut;
bool m_running = false;

bool isRunning()
{
  return m_running;
}

void setRunning(bool running)
{
  m_running = running;
}

void func1(bool isSet)
{
  mut.lock();
  if(!isRunning())
  {
    setRunning(true);
  }
  mut.unlock();
}

int main()
{
  std::thread t1(func1,true);
  std::thread t2(func1,true);
  std::thread t3(func1,true);
  std::thread t4(func1,true);

  t1.join();
  t2.join();
  t3.join();
  t4.join();

  return 0;
}

Edit: Changed !isRunning to !isRunning() (typo)

\$\endgroup\$
6
\$\begingroup\$

In this case, you really just need an atomic variable, and you can do a compare_exchange_strong to set the value once:

#include <thread>
#include <iostream>

std::atomic_bool m_running;
bool wrote[4];

void func1(int num) {
    bool f{ false };
    if (m_running.compare_exchange_strong(f, true))
        wrote[num] = true;
}

int main(){
    std::vector<std::thread> t;

    for (int i = 0; i < 4; i++)
        t.emplace_back(func1, i);

    for (auto &t1 : t)
        t1.join();

    for (bool b : wrote)
        std::cout << std::boolalpha << b << "\n";
}

Given that thread 1 starts running first, and it's not doing very much, chances are that the first thread will do the write to the variable virtually every time. In case you're not familiar with it, atomic_bool is (obviously enough) an atomic Boolean type, and compare_exchange_strong does an atomic comparison and exchange. Specifically, it compares the current value to the first parameter, then if (and only if) they're equal, sets the value to the second parameter. The first parameter has to be passed by reference, because it also writes the new value to that variable. Finally, it returns true if and only if it actually wrote to the variable. In this case I've used that to record which thread actually wrote to the variable, so the final printout should show true once, and false three times.

Looking at the code as you've written it:

void func1(bool isSet)
{
  mut.lock();
  if (!isRunning())
  {
    setRunning(true);
  }
  mut.unlock();
}

You almost certainly want to use an std::lock_guard to automate unlocking the mutex. Right now, the code also ignores the isSet parameter, and always sets the value to true. I'd assume you really want to compare to isSet, and set it to that value if it's not already equal to it. Combining these, the code would come out something like:

void func1(bool isSet) { 
     std::lock_guard g(mut);

     if (isRunning() != isSet)
        setRunning(isSet);
}

That's somewhat simpler, but I still think the atomic_bool with compare_exchange_strong is neater. The atomic variable can degenerate to pretty much what you've done with a mutex in the worst case, but in quite a few cases, it'll be substantially more efficient.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.