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

[Rémi Denis-Courmont]

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.

-- 
Rémi Denis-Courmont