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

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

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/test/std/thread/thread.condition/thread.condition.condvar/wait_for.pass.cpp:50
+           cv.wait_for(lk, wait_end - Clock::now()) == std::cv_status::no_timeout)
         ;
     Clock::time_point t1 = Clock::now();
----------------
What happens with the subtraction when `wait_end < Clock::now()`?
(This could happen because of a libc++ bug; or more likely it could happen because `wait_for` woke spuriously after 249.9ms and then it took us 0.2ms to get around to calling `Clock::now()` again.)

But also, there's no way this code should ever be intentionally waiting 250ms even once. The main thread is just waiting for line 44's `test1=1;` to happen, and then it //immediately// sets `test2=1` and notifies (line 76).

I think the only way that our line 48/49 can ever time out if if it entirely //misses// the main thread's notify due to not being asleep yet. Which actually probably happens a lot. But if the //point// of this test is to time out, then the main thread really shouldn't be notifying //at all ever// (and the timeout should probably be shorter, say 50ms). And if that's //not// the point of this test, then the main thread should be notifying only //after// `f` has gone to sleep, so the notify doesn't get missed. I think the straightforward way to do that is to swap lines 75+76 so that the main thread notifies under the lock. (That way, there's no semantic race condition where `f` spuriously wakes, then `main` notifies (without holding the lock) which races with `f`'s going back to sleep (under the lock).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99175