Closed reuk closed 5 months ago
Ping, just in case this got lost over the festive period.
There's been another point release since I opened this issue.
This is a real bug that causes problems for people running the validator regularly, especially on CI.
It would be great if this fix could be added in the next version of the SDK. Thanks!
Hi, It will be add to the next update, Thanks.
At JUCE, we run the validator in our CI pipelines. We are frequently seeing that the validator hangs and causes the build to time-out. In a recent build on Linux, we saw that the build hung after printing
[ConnectionProxy: Send message on 2nd thread]
, never printing[Succeeded]
, so this test seems to be flaky.I suspect that the use of
condition_variable::wait
is the cause of the hang. If the second thread starts quickly and callsnotify_all
before the first thread callswait
, then thewait
may never be unblocked.Note that it's also possible for
wait
to wake spuriously. If this were to happen before the second thread calledproxy.notify
, it's possible that the testEXPECT_NE (*notifyResult, kResultTrue);
would fail.For these reasons, it's generally recommended to use the form of
wait
that takes a predicate argument, so that the predicate is checked before starting to wait, and on each spurious wake.There are a couple of other issues with this test:
Even though
notifyResult
is atomic, it should only be modified while holding the condition_variable's lock, according to cppreference. This isn't happening at the moment. The condvar is usingm1
, butnotifyResult
is modified underm2
.There's no need to use
notify_all
here. Only one thread is waiting, sonotify_one
is sufficient.