[libcxx-commits] [PATCH] D98334: [libcxx] Fix hang in try_acquire with __atomic_semaphore_base

[Olivier Giroux via Phabricator via libcxx-commits]

__simt__ added inline comments.


================
Comment at: libcxx/include/semaphore:114
+        auto const __test_fn = [=]() -> bool { return try_acquire(); };
         return __libcpp_thread_poll_with_backoff(__test_fn, __libcpp_timed_backoff_policy(), __rel_time);
     }
----------------
pabusse wrote:
> Quuxplusone wrote:
> > So `__libcpp_thread_poll_with_backoff` treats 0=infinite, but `__libcpp_semaphore_wait_timed` (line 167) treats 0=0? That sounds like a recipe for bugs. Is there any appetite to just make libc++ treat 0=0 uniformly across the board?
> > (Then, if someone wants to wait for a billion seconds, they just pass `1000000000` as the timeout — I don't see any practical need to overload `0` with that extra meaning.)
> That's correct.
> 
> I didn't modify the semantics of `__libcpp_thread_poll_with_backoff ` straight away as it is also used in some other contexts with zero-intending-infinite. I agree however that this might be the right change to make.
A large timeout would still mean you have to call it in a loop, whereas the current arrangement avoids that.

But we could make the interface more clear to avoid this kind of bug, I agree we should try.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98334