Fix issue 11519 - race in core.thread unit test #728
Conversation
| sema.notify(); | ||
| semb.wait(); | ||
|
|
||
| Thread.sleep(dur!"msecs"(48)); |
There was a problem hiding this comment.
What this is for? And the 48 magic number.
There was a problem hiding this comment.
It's there to provoke a race with thread_suspendAll, i.e. let the thread spend some time in the critical region to test that it can't be suspended.
I think 48.msecs is a bit excessive for a unit test. I tested it with 1.msecs and it will reliably assert when critical regions aren't respected in thread_suspendAll.
|
I've reset the sleep to 1ms. I wanted to run it at least once with the paranoid 48 ms setting. |
Fix issue 11519 - race in core.thread unit test
|
Thanks |
|
Almost any case of using sleep in a test is doing something wrong. This might be an improvement, but it really should be engineered with appropriate interlock primitives and not on time. In many cases the right posix primitive is a barrier (not sure what that would be on windows). |
We're testing that a thread in a critical region cannot be suspended. The alternative to sleeping would be to burn CPU cycles. Using a mutex/semaphore would result in a deadlock. |
This is a backport of the relevant code from dlang/druntime@98215d6.
https://d.puremagic.com/issues/show_bug.cgi?id=11519