0

This is related to the second answer to this question.

My test code is below. I'm trying to launch a thread, and then to make it stop using the std::atomic_flag. Then the thread should output a number of loop executions and total duration, and stop.

std::atomic_flag keepRunning = ATOMIC_FLAG_INIT;

void F()
{
  keepRunning.test_and_set();
  long long unsigned count = 0;
  const time_t start = time(nullptr);
  while (keepRunning.test_and_set())
  {
    std::cout << '.';
    ++count;
  }
  const time_t duration = time(nullptr) - start;
  std::cout << count << '\t' << duration << std::endl;
}

int main()
{
  std::thread t(F);
  keepRunning.clear();
  t.join();
}

The problem is that the thread doesn't stop.

Why is that?

  • Compiler: g++ (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
  • OS: guest OS Ubuntu 14.04 on the macOS Sierra Ver 10.12.2 host
  • Compilation flags - I tried -O0 and -O4, and it didn't make any difference.
HEKTO
  • 3,876
  • 2
  • 24
  • 45

2 Answers2

2

It does not stop (most of the time) because the loop in F will probably never see a cleared flag.

Assuming that the creation of a thread will take some time, keepRunning.clear() in main is likely to run first. When F finally gets to run, it immediately sets the value and enters a loop that will never see a cleared flag and therefore never quits.

Instead of initially setting the flag value in F, a solution is to initialize it to true. However, std::atomic_flag does not let you do that because of its limited interface (this design is on purpose,std::atomic_flag is supposed to be used as a low-level building block for other primitives).

You could use a std::atomic<bool>, initialize it to true and remove the initial store(true) in F. For demo purposes, I added a sleep_for statement before clearing the flag in main.

std::atomic<bool> keepRunning{true};

void F()
{
  long long unsigned count = 0;
  const time_t start = time(nullptr);
  while (keepRunning)
  {
    std::cout << '.';
    ++count;
  }
  const time_t duration = time(nullptr) - start;
  std::cout << count << '\t' << duration << std::endl;
}

int main()
{
  std::thread t(F);
  std::this_thread::sleep_for(1s); // optional
  keepRunning = false;
  t.join();
}
LWimsey
  • 6,189
  • 2
  • 25
  • 53
1

In your F() you ignore the output from the first keepRunning.test_and_set(). Your attempt to initialize the flag there causes a race with the keepRunning.clear() statement in main(). Depending on which of these statements runs first you either get the intended behavior or have ignored clear() call and a never ending thread.

By the time F() had a chance to run, the flag should be already initialized with the correct value. Moving that initial test_and_set() into main() prevents the race:

std::atomic_flag keepRunning = ATOMIC_FLAG_INIT;

void F()
{
  long long unsigned count = 0;
  const time_t start = time(nullptr);
  while (keepRunning.test_and_set())
  {
    std::cout << '.';
    ++count;
  }
  const time_t duration = time(nullptr) - start;
  std::cout << count << '\t' << duration << std::endl;
}

int main()
{
  keepRunning.test_and_set();
  std::thread t(F);
  keepRunning.clear();
  t.join();
}

Now the flag is effectively only changed in main() and only "read" in F().

NonNumeric
  • 1,079
  • 7
  • 19