Re: quartz: Have thread safety in memallocator

[Rob Shearman]

2008/6/23 Maarten Lankhorst <m.b.lankhorst(a)gmail.com>:
> @@ -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.

-- 
Rob Shearman