[vlc-devel] [vlc-commits] picture_pool: fix race condition

[Thomas Guillem]

On Fri, Oct 31, 2014, at 11:33, Rémi Denis-Courmont wrote:
> Le 2014-10-31 12:54, Thomas Guillem a écrit :
> > On Wed, Oct 29, 2014, at 20:27, Rémi Denis-Courmont wrote:
> >> vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Tue 
> >> Oct 28
> >> 21:30:02 2014 +0200| [9e99ef07827bb2a1796770ad0b12b028b8643ede] |
> >> committer: Rémi Denis-Courmont
> >>
> >> picture_pool: fix race condition
> >>
> >> This makes picture_pool_Get() reentrant.
> >>
> >> > 
> >> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=9e99ef07827bb2a1796770ad0b12b028b8643ede
> >> ---
> >>
> >>  src/misc/picture_pool.c |    5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/misc/picture_pool.c b/src/misc/picture_pool.c
> >> index cfaa541..2a0f677 100644
> >> --- a/src/misc/picture_pool.c
> >> +++ b/src/misc/picture_pool.c
> >> @@ -272,7 +272,9 @@ picture_t *picture_pool_Get(picture_pool_t 
> >> *pool)
> >>              continue;
> >>
> >>          picture_t *picture = pool->picture[i];
> >> -        if (atomic_load(&picture->gc.refcount) > 0)
> >> +        uintptr_t refs = 0;
> >> +
> >> +        if (!atomic_compare_exchange_strong(&picture->gc.refcount,
> >> &refs, 1))
> >>              continue;
> >>
> >>          if (pool->pic_lock != NULL && pool->pic_lock(picture) != 0)
> >> @@ -281,7 +283,6 @@ picture_t *picture_pool_Get(picture_pool_t 
> >> *pool)
> >>          /* */
> >>          picture->p_next = NULL;
> >>          picture->gc.p_sys->tick = pool->tick++;
> >> -        picture_Hold(picture);
> >>          return picture;
> >>      }
> >>      return NULL;
> >>
> >
> > This patch break software rendering on android: no picture rendered.
> 
> This change is quite simple and the affected code is covered by unit 
> tests so I have serious doubts that it breaks _anything_.
> 
> If you think the code is wrong, please explain what is wrong or better 
> yet write a test case.

I'm not saying the code is wrong, I'm saying it breaks android
rendering.
I don't understand well that part of the code, so I trust you and your
code.
But maybe, you can help me fix android rendering ? Do you have a hint ?

> 
> -- 
> Rémi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel