quartz: Fix deadlocks in pullpin.

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;
 }

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,

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);