[PATCH 2/3] winecoreaudio: Implement the lock-free callback design.

[Joerg-Cyril.Hoehle at t-systems.com]

Hi,

Andrew Eikum wrote:
>I wonder if it's appropriate for code freeze.
You choose:
- either 1.4 keeps the old code as is and suffers random dead-locks, or
- 1.4 integrates code void of race-conditions on the basis of a solid design.

>When working on this patch, were you able to identify
>specifically where the contention is?
The "10 liner test hack" I mentioned in bug #29657
was simply to remove OSSpinLock inside the callbacks.
No more dead-lock.  It worked quite well (over hundred successful
test iterations to verify the hypothesis), but it was a
time bomb and eventually crashed.

Unlike a 10 line hack, a proper solution costs a hundred lines.


>The idea of the InTransition state was to handle the lock
>release in mid-call. Can we add a check for that state in
>the callbacks to solve this problem less intrusively?
and then do what?

I've never experienced code that works releasing locks mid-call.
Quite to the contrary, I'm convinced that such patterns are
a sure indicator of a design flaw and bound to cause trouble.

Of the same vein, one liners like
EnterCS
This-> playing = StateStopped;
LeaveCS
are suspicious too.  What does that guarantee?

You're right that InTransition is no more needed with my patch.
A post code-freeze patch can clean up.  More specifically, I've kept
it so far because of Wait(event) in Stop, outside the lock.


While winealsa would benefit too from a lock-free player as
sketched in bug #29531, I'm not considering submitting
it for 1.4.  The difference is that some apps may suffer audio
glitches with winealsa, however any app can suffer a random
freeze with winecoreaudio.  (Well, perhaps I'll write it meanwhile
and if it exhibits significant improvements, submit it anyway).

Next on my list for 1.4 is #28388.  There too, a random dead-lock
in any MIDI playing app is not acceptable IMHO.

GetPosition is also crucial for 1.4, regardless of code-freeze.

Regards,
	Jörg Höhle