[libcxx-commits] [PATCH] D153441: [libc++] Implement `std::condition_variable_any::wait[_for/until]` overloads that take `stop_token`

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_token_pred.pass.cpp:134
+    auto thread           = support::make_test_thread([&]() {
+      std::this_thread::sleep_for(2ms);
+      ss.request_stop();
----------------
Mordante wrote:
> I wonder here and at other places whether 2 ms is long enough.
> Are we sure this is changed after the `wait_for` is really in it's loop? Or can the flag be set before entering the loop.
> 
> Would it make sense to validate how often the predicate is executed? When it's less than 2 the loop was executed 0 times.
If the thread creation is really slow, this test could be vacuous (it would test the same as the test that does `request_stop()` before we `wait_for`). Would it make sense to instead do something like:

```
auto thread = support::make_test_thread([&]() {
    // wait until we're ready
    some_atomic.wait(...);
    ss.request_stop();

    while (!done) {
        cv.notify_all();
        std::this_thread::sleep_for(2ms);
      }
});

std::same_as<bool> auto r = cv.wait_for(lock, ss.get_token(), 1h, [&]() {
  some_atomic.notify_all(); // here signal that we're ready for the other thread to request a stop
  return false;
});
assert(!r);
done = true;
thread.join();
```

Does this make sense at all? That way the `pred` will be the one to trigger the fact that we `request_stop()` on the other thread. Would that be more robust?


================
Comment at: libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_token_pred.pass.cpp:79
+
+  // stop request comes while waiting
+  {
----------------
Whatever we do for the other test we should do here too.


================
Comment at: libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_token_pred.pass.cpp:129
+
+  // stop request comes while waiting
+  {
----------------
And here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153441