From f82f8355523103edb4a544d894f2ed2e6ef804c5 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 7 Apr 2008 15:45:51 -0700 Subject: [PATCH] quartz: Fix deadlocks in pullpin --- dlls/quartz/parser.c | 73 +++++++++++++++++++++---------------------------- dlls/quartz/pin.c | 57 ++++++++++++++++++++++++++++++-------- dlls/quartz/pin.h | 15 +++++++--- 3 files changed, 87 insertions(+), 58 deletions(-) diff --git a/dlls/quartz/parser.c b/dlls/quartz/parser.c index aec49eb..670a0c3 100644 --- a/dlls/quartz/parser.c +++ b/dlls/quartz/parser.c @@ -213,9 +213,11 @@ static HRESULT WINAPI Parser_Stop(IBaseFilter * iface) { HRESULT hr; ParserImpl *This = (ParserImpl *)iface; + PullPin *pin = (PullPin *)This->ppPins[0]; TRACE("()\n"); + EnterCriticalSection(&pin->thread_lock); EnterCriticalSection(&This->csFilter); { if (This->state == State_Stopped) @@ -228,60 +230,40 @@ static HRESULT WINAPI Parser_Stop(IBaseFilter * iface) LeaveCriticalSection(&This->csFilter); hr = PullPin_StopProcessing(This->pInputPin); + LeaveCriticalSection(&pin->thread_lock); return hr; } static HRESULT WINAPI Parser_Pause(IBaseFilter * iface) { HRESULT hr = S_OK; - BOOL bInit; ParserImpl *This = (ParserImpl *)iface; - + PullPin *pin = (PullPin *)This->ppPins[0]; + TRACE("()\n"); + EnterCriticalSection(&pin->thread_lock); EnterCriticalSection(&This->csFilter); + + if (This->state == State_Paused) { - if (This->state == State_Paused) - { - LeaveCriticalSection(&This->csFilter); - return S_OK; - } - bInit = (This->state == State_Stopped); - This->state = State_Paused; + LeaveCriticalSection(&This->csFilter); + return S_OK; } - LeaveCriticalSection(&This->csFilter); - if (bInit) + if (This->state == State_Stopped) { - unsigned int i; - - if (SUCCEEDED(hr)) - hr = PullPin_InitProcessing(This->pInputPin); - - if (SUCCEEDED(hr)) - { - for (i = 1; i < This->cStreams + 1; i++) - { - LONGLONG duration; - DOUBLE speed; - - IMediaSeeking_GetDuration((IMediaSeeking *)&This->mediaSeeking, &duration); - IMediaSeeking_GetRate((IMediaSeeking *)&This->mediaSeeking, &speed); - OutputPin_DeliverNewSegment((OutputPin *)This->ppPins[i], 0, duration, speed); - OutputPin_CommitAllocator((OutputPin *)This->ppPins[i]); - } - - /* FIXME: this is a little hacky: we have to deliver (at least?) one sample - * to each renderer before they will complete their transitions. We should probably - * seek through the stream for the first of each, rather than do it this way which is - * probably a bit prone to deadlocking */ - hr = PullPin_StartProcessing(This->pInputPin); - } + LeaveCriticalSection(&This->csFilter); + hr = IBaseFilter_Run(iface, -1); + EnterCriticalSection(&This->csFilter); } - /* FIXME: else pause thread */ + This->state = State_Paused; + + LeaveCriticalSection(&This->csFilter); if (SUCCEEDED(hr)) hr = PullPin_PauseProcessing(This->pInputPin); + LeaveCriticalSection(&pin->thread_lock); return hr; } @@ -290,10 +272,13 @@ static HRESULT WINAPI Parser_Run(IBaseFilter * iface, REFERENCE_TIME tStart) { HRESULT hr = S_OK; ParserImpl *This = (ParserImpl *)iface; + PullPin *pin = (PullPin *)This->ppPins[0]; + int i; TRACE("(%s)\n", wine_dbgstr_longlong(tStart)); + EnterCriticalSection(&pin->thread_lock); EnterCriticalSection(&This->csFilter); { if (This->state == State_Running) @@ -330,6 +315,7 @@ static HRESULT WINAPI Parser_Run(IBaseFilter * iface, REFERENCE_TIME tStart) This->state = State_Running; } LeaveCriticalSection(&This->csFilter); + LeaveCriticalSection(&pin->thread_lock); return hr; } @@ -337,31 +323,33 @@ static HRESULT WINAPI Parser_Run(IBaseFilter * iface, REFERENCE_TIME tStart) static HRESULT WINAPI Parser_GetState(IBaseFilter * iface, DWORD dwMilliSecsTimeout, FILTER_STATE *pState) { ParserImpl *This = (ParserImpl *)iface; + PullPin *pin = (PullPin *)This->ppPins[0]; + HRESULT hr = S_OK; TRACE("(%d, %p)\n", dwMilliSecsTimeout, pState); + EnterCriticalSection(&pin->thread_lock); EnterCriticalSection(&This->csFilter); { *pState = This->state; } LeaveCriticalSection(&This->csFilter); - /* FIXME: this is a little bit unsafe, but I don't see that we can do this - * while in the critical section. Maybe we could copy the pointer and addref in the - * critical section and then release after this. - */ if (This->pInputPin && (PullPin_WaitForStateChange(This->pInputPin, dwMilliSecsTimeout) == S_FALSE)) - return VFW_S_STATE_INTERMEDIATE; + hr = VFW_S_STATE_INTERMEDIATE; + LeaveCriticalSection(&pin->thread_lock); - return S_OK; + return hr; } static HRESULT WINAPI Parser_SetSyncSource(IBaseFilter * iface, IReferenceClock *pClock) { ParserImpl *This = (ParserImpl *)iface; + PullPin *pin = (PullPin *)This->ppPins[0]; TRACE("(%p)\n", pClock); + EnterCriticalSection(&pin->thread_lock); EnterCriticalSection(&This->csFilter); { if (This->pClock) @@ -371,6 +359,7 @@ static HRESULT WINAPI Parser_SetSyncSource(IBaseFilter * iface, IReferenceClock IReferenceClock_AddRef(This->pClock); } LeaveCriticalSection(&This->csFilter); + LeaveCriticalSection(&pin->thread_lock); return S_OK; } diff --git a/dlls/quartz/pin.c b/dlls/quartz/pin.c index f762eb8..8ec39be 100644 --- a/dlls/quartz/pin.c +++ b/dlls/quartz/pin.c @@ -618,16 +618,15 @@ HRESULT WINAPI InputPin_BeginFlush(IPin * iface) HRESULT hr; TRACE("() semi-stub\n"); - /* Assign this outside the critical section so that _Receive loops can be broken */ - This->flushing = 1; - EnterCriticalSection(This->pin.pCritSec); + This->flushing = 1; if (This->fnCleanProc) This->fnCleanProc(This->pin.pUserData); hr = SendFurther( iface, deliver_beginflush, NULL, NULL ); LeaveCriticalSection(This->pin.pCritSec); + return hr; } @@ -965,7 +964,7 @@ HRESULT WINAPI OutputPin_Disconnect(IPin * iface) hr = S_FALSE; } LeaveCriticalSection(This->pin.pCritSec); - + return hr; } @@ -1200,6 +1199,9 @@ static HRESULT PullPin_Init(const IPinVtbl *PullPin_Vtbl, const PIN_INFO * pPinI pPinImpl->rtStop = ((LONGLONG)0x7fffffff << 32) | 0xffffffff; pPinImpl->state = State_Stopped; + InitializeCriticalSection(&pPinImpl->thread_lock); + pPinImpl->thread_lock.DebugInfo->Spare[0] = (DWORD_PTR)( __FILE__ ": PullPin.thread_lock"); + return S_OK; } @@ -1343,6 +1345,8 @@ ULONG WINAPI PullPin_Release(IPin * iface) if(This->pReader) IAsyncReader_Release(This->pReader); CloseHandle(This->hEventStateChanged); + This->thread_lock.DebugInfo->Spare[0] = 0; + DeleteCriticalSection(&This->thread_lock); CoTaskMemFree(This); return 0; } @@ -1555,6 +1559,8 @@ HRESULT PullPin_PauseProcessing(PullPin * This) HRESULT PullPin_StopProcessing(PullPin * This) { + TRACE("(%p)->()\n", This); + /* if we are connected */ if (This->pAlloc && This->hThread) { @@ -1587,24 +1593,51 @@ HRESULT WINAPI PullPin_EndOfStream(IPin * iface) HRESULT WINAPI PullPin_BeginFlush(IPin * iface) { PullPin *This = (PullPin *)iface; - FIXME("(%p)->() stub\n", iface); + TRACE("(%p)->()\n", iface); + + EnterCriticalSection(&This->thread_lock); + { + if (This->state == State_Running) + PullPin_PauseProcessing(This); + + PullPin_WaitForStateChange(This, INFINITE); + } + LeaveCriticalSection(&This->thread_lock); + + EnterCriticalSection(This->pin.pCritSec); + { + This->fnCleanProc(This->pin.pUserData); - SendFurther( iface, deliver_beginflush, NULL, NULL ); + SendFurther( iface, deliver_beginflush, NULL, NULL ); + } + LeaveCriticalSection(This->pin.pCritSec); - if (This->state == State_Running) - return PullPin_PauseProcessing(This); return S_OK; } HRESULT WINAPI PullPin_EndFlush(IPin * iface) { - FILTER_STATE state; PullPin *This = (PullPin *)iface; - FIXME("(%p)->() stub\n", iface); + TRACE("(%p)->()\n", iface); + + EnterCriticalSection(&This->thread_lock); + { + FILTER_STATE state; + IBaseFilter_GetState(This->pin.pinInfo.pFilter, INFINITE, &state); + + if (state == State_Running && This->state == State_Paused) + PullPin_StartProcessing(This); + + PullPin_WaitForStateChange(This, INFINITE); + } + LeaveCriticalSection(&This->thread_lock); + + EnterCriticalSection(This->pin.pCritSec); SendFurther( iface, deliver_endflush, NULL, NULL ); + LeaveCriticalSection(This->pin.pCritSec); - return IBaseFilter_GetState(This->pin.pinInfo.pFilter, INFINITE, &state); + return S_OK; } HRESULT WINAPI PullPin_NewSegment(IPin * iface, REFERENCE_TIME tStart, REFERENCE_TIME tStop, double dRate) @@ -1624,7 +1657,7 @@ static const IPinVtbl PullPin_Vtbl = PullPin_QueryInterface, IPinImpl_AddRef, PullPin_Release, - OutputPin_Connect, + InputPin_Connect, PullPin_ReceiveConnection, IPinImpl_Disconnect, IPinImpl_ConnectedTo, diff --git a/dlls/quartz/pin.h b/dlls/quartz/pin.h index f97a33d..19c5246 100644 --- a/dlls/quartz/pin.h +++ b/dlls/quartz/pin.h @@ -98,6 +98,11 @@ typedef struct PullPin double dRate; FILTER_STATE state; BOOL stop_playback; + + /* Any code that touches the thread must hold the thread lock, + * lock order: thread_lock and then the filter critical section + */ + CRITICAL_SECTION thread_lock; } PullPin; /*** Constructors ***/ @@ -164,12 +169,14 @@ HRESULT WINAPI MemInputPin_ReceiveCanBlock(IMemInputPin * iface); HRESULT WINAPI PullPin_ReceiveConnection(IPin * iface, IPin * pReceivePin, const AM_MEDIA_TYPE * pmt); HRESULT WINAPI PullPin_QueryInterface(IPin * iface, REFIID riid, LPVOID * ppv); ULONG WINAPI PullPin_Release(IPin * iface); -HRESULT PullPin_InitProcessing(PullPin * This); -HRESULT PullPin_StartProcessing(PullPin * This); -HRESULT PullPin_StopProcessing(PullPin * This); -HRESULT PullPin_PauseProcessing(PullPin * This); HRESULT WINAPI PullPin_EndOfStream(IPin * iface); HRESULT WINAPI PullPin_BeginFlush(IPin * iface); HRESULT WINAPI PullPin_EndFlush(IPin * iface); HRESULT WINAPI PullPin_NewSegment(IPin * iface, REFERENCE_TIME tStart, REFERENCE_TIME tStop, double dRate); + +/* Thread interaction functions: Hold the thread_lock before calling them */ +HRESULT PullPin_InitProcessing(PullPin * This); +HRESULT PullPin_StartProcessing(PullPin * This); +HRESULT PullPin_StopProcessing(PullPin * This); +HRESULT PullPin_PauseProcessing(PullPin * This); HRESULT PullPin_WaitForStateChange(PullPin * This, DWORD dwMilliseconds); -- 1.5.4.1