[libcxx-commits] [PATCH] D114119: [libcxx] Fix potential lost wake-up in counting semaphore

Wed Dec 1 08:56:29 PST 2021

ldionne added inline comments.

================
Comment at: libcxx/test/std/thread/thread.semaphore/lost_wakeup.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
fwolff wrote:
> ldionne wrote:
> > Do you think this test is the reason why the CI nodes timed out after running for 2 hours? It seems like it's not consistently slow (I just restarted the macOS CI and it finished quickly, but before that it froze for 2 hours, which I've never seen). https://buildkite.com/llvm-project/libcxx-ci/builds/6835#4278d152-fa4d-482f-a12e-e9697caa99e3
> I don't see what could be causing this, but I agree that it looks suspicious.
> 
> The fix itself should be uncontroversial, though, at least as far as correctness is concerned, because we are always waking up //at least as many// threads as before, so if anything, we're making deadlocks //less// likely to occur.
> 
> What options do we have regarding this test? Could the problem also lie with, say, `std::barrier`?
I pulled the patch down to run it locally. First time went well, second time -- I'm still running the test after a couple minutes. It looks like it's hanging for some reason.

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

https://reviews.llvm.org/D114119