[libcxx-commits] [libcxx] [libc++] fix `counting_semaphore` lost wakeups (PR #79265)

via libcxx-commits libcxx-commits at lists.llvm.org
Wed Jan 31 09:20:50 PST 2024


================
@@ -95,19 +96,22 @@ public:
     _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(
         __update <= _LIBCPP_SEMAPHORE_MAX - __old, "update is greater than the expected value");
 
-    if (__old > 0) {
-      // Nothing to do
-    } else if (__update > 1)
+    if (__old == 0) {
----------------
huixie90 wrote:

> I agree, either approach should fix the problem. I wonder which one is better though. My gut feeling is that the first one is more in the "spirit of futex", i.e. that futex operations should be limited to the contended case. The downside of the second approach is that every `notify()` could lead to a system call, which might be bad for performance. I guess one would have to benchmark.
> 
> Sadly the ABI is locked in -- if one had an additional "waiters" field it would be possible to call `notify_one()`/`notify_all()` in an optimal way. A "lesser" ABI break could be to stuff both the number of waiters and the "permits" into the 64 bit of the semaphore (on 64 bit systems). Then, only `max()` and `least_max_value` would be affected. But who really needs 63/64 bit of semaphore range?

but we can't change the default template parameter number, can we?
`counting_semaphore<>` needs to stay `counting_semaphore<numeric_limits<ptrdiff_t>::max()>` , which means that all bits are used

https://github.com/llvm/llvm-project/pull/79265


More information about the libcxx-commits mailing list