Hi Jörg,
Patch makes sense to me. I'll put together some patches for the other drivers, unless you beat me to it ;)
Thanks, Andrew
On 06/17/2011 10:13 AM, Joerg-Cyril.Hoehle@t-systems.com wrote:
Hi,
my patch is not the final word about robust design.
Consider what happens when DeleteTimerQueueTimer(g_timer_q, This->timer, INVALID_HANDLE_VALUE) cannot allocate its CompletionEvent. The function will return with an (ignored) error and not stop the timers. Do you see a check in wineoss or winecoreaudio? The later Audio_Release will destroy critical sections and free memory while the timers are still running!
A tiny allocation failure => big crash
Deallocators should not fail when resources are scarce.
A more robust design would see Audio_Start allocate the completion event that DeleteTimerQ requires and not start if it can't be sure to stop. Why require a completion event? Because there's *no* other way of telling whether a callback might still be invoked. Even sleeping for more than the timer period is no guarantee rather than mere heuristics that works on a slightly loaded system.
Back to my patch. It's funny to think that it would mostly work without error checking: DeleteTimerQ(event) does something with event NULL or not, Wait(NULL) would immediately return and DeleteTimerQ always signals the event when successful.
Regards, Jörg Höhle