[MPlayer-dev-eng] [PATCH] threaded cache, round 2
Dan Oscarsson
Dan.Oscarsson at tieto.com
Tue Jan 3 09:58:04 CET 2012
mån 2012-01-02 klockan 22:06 +0000 skrev Martin Simmons:
> >>>>> On Mon, 02 Jan 2012 17:59:19 +0100, Dan Oscarsson said:
> >
> > +#ifdef PTHREAD_CACHE
> > + if (!((cache_vars_t *)s->cache_data)->do_work) {
> > + pthread_mutex_lock(&((cache_vars_t *)s->cache_data)->go_ahead_mutex);
> > + ((cache_vars_t *)s->cache_data)->do_work = 1;
> > + pthread_mutex_unlock(&((cache_vars_t *)s->cache_data)->go_ahead_mutex);
> > + pthread_cond_signal(&((cache_vars_t *)s->cache_data)->go_ahead);
>
> The last two lines are in the wrong order -- to avoid race conditions, you
> must signal while holding the lock.
No, not from my analysis. If cache thread detects that do_work is 1
before the signal is sent, it does not matter. Either the cache thread
detects that do_work is 1 and starts to work, or it locks the mutex
before do_work is set to 1, and waits for signal instead.
>
>
> > +#ifdef PTHREAD_CACHE
> > + if (!s->do_work) {
> > + pthread_mutex_lock(&s->go_ahead_mutex);
> > + s->do_work = 1;
> > + pthread_mutex_unlock(&s->go_ahead_mutex);
> > + pthread_cond_signal(&s->go_ahead);
>
> Ditto.
>
>
> > +#ifdef PTHREAD_CACHE
> > + if (!s->do_work) {
> > + pthread_mutex_lock(&s->go_ahead_mutex);
> > + while (!s->do_work) pthread_cond_wait(&s->go_ahead, &s->go_ahead_mutex);
> > + pthread_mutex_unlock(&s->go_ahead_mutex);
> > + }
> > + s->do_work = 0;
>
> Set s->do_work to 0 while holding the lock inside the if, also to avoid race
> conditions.
Quite often it is so, but not in this case (unless my analysis is
wrong). When cache thread sets do_work to 0 above, it is going to do
some work (reading data and command processing), so it does not matter
if do_work is set to 1 by main thread and that is erased by this setting
to zero.
I moved pthread_cond_signal and the setting of do_work = 0 outside the
mutex lock to reduce time spent inside the lock as I it looks like the
cannot be a race condition in this case.
We can move them back in even if nobody can show a case where it gets
wrong, just to be sure.
Dan
More information about the MPlayer-dev-eng
mailing list