16-May-2013 02:21, Idan Arye пишет:
On Wednesday, 15 May 2013 at 21:52:40 UTC, Dmitry Olshansky wrote:
Sleep in concurrency code that aims to synchronize something is always
bad idea (=BUG).
Don't say "always". Only a Sith deals in absolutes. Sure, `sleep()` in
concurrent code is a bad idea because it can cause race conditions.
Well I embraced the power of the dark side long ago ;)
And I can confidently say that sleep doesn't cause anything reliably at
all. Try measuring time with it (e.g. in ms range) and you'll see it :)
But
I *want* to (almost)cause a race condition here
You can't "cause" a race condition - it's there by definition of code.
What you can cause is a data corruption that happened with one bad
interleave of reads/writes because it was possible.
- that's the whole point
of this unit test, to guarantee that the singleton implementation is
immune to race conditions.
I can't see how suspending all threads by 500ms + arbitrary amount of
time of on the order of one context switch (or more depends on how
system is busy) will do that. Sleep basically says wake me up no sooner
then 500ms (i.e. put back into active threads queue), it doesn't
guarantee any kind of timing or priority.
Even with proper timings launching a bunch of threads at almost the same
time still doesn't guarantee it. You are far better off with solid
formal prof in this case (that is there).
What you might be seeing is that if all of threads on idle system
preform context switch and then switch back after sleeping X ms they
might collide.
Crashing your car into a brick wall is usually a bad idea, but not
always.
I'm not seeing this test do anything useful anyway - you know there is a
race condition and then you try to test to see if it works as just as if
there is none.
It's more like unscrewing a bunch of bolts and seeing if the car manages
it to the other town. It might or not depending on how the driver rides
it and on a ton of other chances - truth be told it's blind luck almost.
Even if you unscrew the same bolts exactly the same way the 2nd car will
not ride exactly as long a distance as 1st one.
Sleep is a tool used to give up thread execution resources to avoid
spinning on something wasting cycles. Use semaphore or analogous
primitives to coordinate, see other posts.
In my second version of the unit test(after reading Diggory's comment) I
used a `Barrier` to coordinate, and it gave me 50% accuracy - That is,
when I took the synchronization out of the implementation the unit test
failed 50% of the times. I still need the `sleep()` to make sure the
threads are getting mixed.
And if your PC was compressing video or serving some HTTP you'll have
even less no matter how you try to run them I guess...
But if you like it I think Thread.yield will work just as well, it will
cause threads to do the context switch.
One word - it's a stress test (smoke test). It should be a kind of
loop that executes for as long as you can allow it. Days later if it
didn't fail usually you either decide that it should be "good enough"
or keep running them.
Why in the world would I want to make a unit test for days, waiting for
a race condition that might not happen, when I can easily force a race
condition?
Sleep doesn't guarantee it. It causes context switch though and that
might be what you want. Maybe creating a bunch of threads as suspended
and then waking them all up could get close to that.
Another problem is about expecting something definite out of race
condition. Yes, here you are getting away with single atomic read/write
of pointer simply because of x86 architecture.
Technically what you'll can see with a race condition is undefined (not
speaking of this simple example on x86 that IMO is pointless anyway).
Thus trying to catch it in more complex situation will require more then
just putting a sleep(xyz) before a function call.
Imagine trying to test a lock-free collection ? You still need to lauch
many threads and somehow try to schedule them funky. Even then I don't
see how 1 single-shot can be reliable there.
--
Dmitry Olshansky
