Re: quartz: Have thread safety in memallocator

[Rob Shearman]

2008/6/23 Maarten Lankhorst :
> @@ -286,19 +287,24 @@ static HRESULT WINAPI BaseMemAllocator_GetBuffer(IMemAllocator * iface, IMediaSa
>  *pSample = NULL;
>
> - if (!This->bCommitted)
> -
>    return VFW_E_NOT_COMMITTED;
> - EnterCriticalSection(This->pCritSect);
> - if (!This->bCommitted || This->bDecommitQueued)
> -
>    hr = VFW_E_NOT_COMMITTED;
> - else
> -
>    ++This->lWaiting;
> - LeaveCriticalSection(This->pCritSect);
> - if (FAILED(hr))
> -
>    return hr;
>
> - This->lWaiting++;
> if (WaitForSingleObject(This->hSemWaiting, (dwFlags & AM_GBF_NOWAIT) ? 0 : INFINITE) != WAIT_OBJECT_0)
> {
> -
>    This->lWaiting--;
> -
>    InterlockedDecrement(&This->lWaiting);
>    return VFW_E_TIMEOUT;
> }
> -
>
> This->lWaiting--;
>
> EnterCriticalSection(This->pCritSect);
> {
> -
>    --This->lWaiting;
>    if (!This->bCommitted)
>        hr = VFW_E_NOT_COMMITTED;
>    else if (This->bDecommitQueued)

Just because you increment a variable inside a critical section, it
doesn't mean that it's safe against another thread using
InterlockedIncrement/Decrement on it. Either only change the variable
inside a critical section or only change it using interlocked
functions.