[libcxx-commits] [PATCH] D99175: [libcxx] [test] Make the condvar wait_for tests less brittle

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

> You claim "Then the main thread immediately sets test2=1 and notifies" No, it doesn't always.

Ah, I did miss that main did //two// tests (one without timeout, then one with timeout). You're definitely right that in the one expecting a timeout, spurious wakeups can prolong the test indefinitely. Setting an "end time" as you've done //is// probably the best solution for that.

If I rewrote this test, I guess it'd look something like this. I admit the do-while is a little fiddly, even if you replace it with a `while`. Getting rid of the `runs` variable is an unmitigated win.

  void f(bool expect_timeout)
  {
      typedef std::chrono::system_clock Clock;
      typedef std::chrono::milliseconds milliseconds;
      std::unique_lock<std::mutex> lk(mut);
      assert(test2 == 0);
      test1 = 1;
      cv.notify_one();
      Clock::time_point t0 = Clock::now();
      Clock::time_point t_end = t0 + milliseconds(100);
      std::cv_status status;
      do {
          Clock::time_point t_now = Clock::now();
          if (t_now >= t_end) break;
          status = cv.wait_for(lk, t_end - t_now);
      } while (test2 == 0 && status == std::cv_status::no_timeout);
      if (expect_timeout)
      {
          assert(status == std::cv_status::timeout);
          assert(test2 == 0);
          assert(Clock::now() >= t_end);
      }
      else
      {
          assert(status == std::cv_status::no_timeout);
          assert(test2 == 1);
          assert(Clock::now() < t_end); // in fact practically instantaneous
      }
  }
  
  int main(int, char**)
  {
      {
          std::unique_lock<std::mutex> lk(mut);
          std::thread t = support::make_test_thread(f, false);
  [...]


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99175/new/

https://reviews.llvm.org/D99175