[MPlayer-dev-eng] [PATCH] Fix dead lock in ao_macosx when switch stream format.

[Ulion]

2007/11/5, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> Hello,
> On Mon, Nov 05, 2007 at 09:36:02PM +0800, Ulion wrote:
> > 2007/11/5, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > > On Mon, Nov 05, 2007 at 07:59:25PM +0800, Ulion wrote:
> > > Secondly, even after your change, the callback might use a condition
> > > that has already been destroyed.
> > > Which probably is the thing that causes the deadlock in the first place
> > > (using a lock that has been destroyed), at least as I understand you
> > > description.
> >
> > You got wrong meaning, with gdb to check it on my a little slow
> > powerpc system, the change stream and callback take a time longer than
> > 0.5 second, but the format detection in the loop found the format
> > already changed, it then to remove the listener, the remove call is
> > waiting the listener callback return, but at mean time the listener is
> > blocking and wait the mutex lock, so dead locked.
>
> Then why not release the lock before calling the remove function?
> And if that function waits for the callback that should be documented
> and the lock and condition can be released immediately after, thus also
> avoiding the missing destroy on error.

Yes, we can move the unlock before remove listener, even just after
the wait_cond call, since the mutex is useless to us, the only useage
of the mutex is as a necessary parameter of pthread_cond_timedwait,
indeed we only need the condition and siginal it. Because the same
reason, we should also remove the mutex code in the listener callback
just like my patch did.

>
> > So if we remove the wait mutex in the callback, here will be no
> > deadlock, and after the listener return, the remove call wil finished,
> > everything is ok.
>
> I am not yet convinced this isn't a ugly hack instead of a proper fix.
> I also think that such a solution means that a conditional is required
> whereas my solution should allow using the more lightweight signals if
> someone cares (I doubt it, this looks hardly performance-critical in any
> way, and portability does not really matter either).
>

The mutex lock is useless, don't you think so? Since it's useless,
remove it will be a proper fix, leave the lock code in callback will
keep some dead lock risk if on some platform the get format call after
cond wait also blocked because the format change callback didn't
return. The callback singal caller is only one, and just need notify
us format changed, need not grab the mutex lock, isn't it?


-- 
Ulion