deadlock in mmdevapi AudioClient_Stop (winealsa.drv/mmdevdrv.c)

Maarten Lankhorst m.b.lankhorst at
Wed Jun 15 03:07:28 CDT 2011

Hi Joerg,

Op 15-06-11 10:00, Joerg-Cyril.Hoehle at schreef:
> Hi,
> please try and convince me that a deadlock cannot happen.
> AudioClient_Start calls
>  CreateTimerQueueTimer
>    which creates a new thread periodically invoking the timer callback.
> The callback invokes
> alsa_push_buffer_data()
>  which ceases the audio client object lock (This->lock).
>  AudioClient_Stop performs
>   EnterCriticalSection(&This->lock); then
>   DeleteTimerQueueTimer(g_timer_q, This->timer, INVALID_HANDLE_VALUE);
> Now imagine Stop is invoked, EnterCriticalSection executed then the thread is preempted.
> Suppose the timer callback kicks in: alsa_push_buffer_data will block on This->lock.
> When the thread is resumed, DeleteTimerQueue will hang forever waiting for all timer callbacks to come to an end.
> My opinion is that critical sections are great to prevent concurrent execution while
> things are running, but they are hell to tear down properly.
> IMHO, LeaveCriticalSection + Re-EnterCriticalSection around DeleteTimerQueue is not a solution.
> I consider using Leave+EnterCS around anything that can wait/block an anti-pattern
> (there are a few places like that in Wine). I believe a "restart op" is needed in general
> (like a "restart transaction if it failed" in databases) as anything could have happened while waiting.
You're right, This->timer should be set to NULL inside the lock,
and DeleteTimerQueueTimer should be run afterwards.


More information about the wine-devel mailing list