Re: DirectShow base classes and the possible deadlocks

On Tue, 26 Apr 2005 04:32:03 -0700, "Cyril"
<Cyril@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote:

>Hi,
>
> I've analysed the current implementation of the DirectShow interfaces.
>I've checked them twice, and I now think that there is still a lot of
>"potential" deadlocks.
>Let me explain (before flaming):
> When using CSource and CSourceStream base classes, whenever you start the
>graph, you call <filter>::Run (not directly, but it is called).
>1) CBaseFilter::Run locks the filter lock (FilterLock)
>2) CBaseFilter::Run then can call CBaseFilter::Pause
>3) CBaseFilter::Pause locks the filter lock (okay, it's recursive locking,
>but this is a bad design)
>4) CBaseFilter::Pause will lock the filter's State lock (FilterStateLock)
>twice (by calling GetPin and GetPinCount) and release them
>5) CBaseFilter::Pause calls Pin::Active method
>6) CSourceStream::Active locks (in that order) :
> 1) FilterStateLock
> 2) FilterLock (already locked, but because it's done by the same thread
>it's OK)
> 3) AMThread::AccessLock
> 4) Wait infinite for AMThread::EventComplete
>
>Okay, now let's see when stopping the graph:
>1) CBaseFilter::Stop locks the filter lock (FilterLock)
>2) CBaseFilter::Stop will lock the filter's State lock (FilterStateLock)
>twice (by calling GetPin and GetPinCount) and release them
>3) CBaseFilter::Stop call Pin::Inactive method
>4) CSourceStream::Inactive locks (in that order) :
> 1) FilterStateLock
> 3) AMThread::AccessLock
> 4) Wait infinite for AMThread::EventComplete
> etc...
>
>So if you look closely to the sequence above, in one case the locking order
>is:
>FilterLock->FilterStateLock (Inactive)
>And in the other case it is:
>FilterStateLock->FilterLock (Active)
>
>So if I'm trying to stop the filter in a thread B, and start it back in
>thread A, it might deadlock (thread B holding FilterLock and waiting for
>FilterStateLock, and thread A holding FilterStateLock and waiting for
>FilterLock).
>
>In usual (most) software, it is the same application thread that start and
>stop the filter, so the lock is recursively acquired, and the deadlock never
>occurs.
>The filter state doesn't prevent the lock (just write it down for each step
>you'll see)
>
>Of course, this only happen when you call Active directly (step 6 above).
>A simple solution to this would be to move the 6-2) step above before the
>6-1).
>This is done by moving the
>296: if(m_pFilter->IsActive()) {
> return S_FALSE; // succeeded, but did not allocate resources (they
>already exist...)
> }
>in BaseClasses\Source.cpp up to the line 291

I'm not sure there is a problem here.

Look at your analysis above. Both Run() and Stop() first take
FilterLock, then they take FilterStateLock. Nobody is locking
FilterStateLock without already holding FilterLock.

You're not supposed to call Active() or Inactive() directly --
CBaseFilter calls it for you, and it is holding FilterLock when it does.

So, according to your analysis, what will happen if one thread tries to
start the filter, and another thread tries to stop the filter, is:

Thread A:

- Calls Run()
- Run() grabs FilterLock
- Run() holds FilterLock until finished

Thread B:

- Calls Stop()
- Stop() tries to grab FilterLock, but cannot get it until Run() is
finished

Consequently, Active() and Inactive() cannot be called at the same time.

Or am I misunderstanding you?

--
New to newsgroups? Read: http://dev.6581.com/newsgroups.html
.

Loading