[vlc-devel] [PATCH] android: threads support

[Rafaël Carré]

Le 05/10/2012 09:03, Rémi Denis-Courmont a écrit :
> On Thu, 04 Oct 2012 23:22:09 +0200, Rafaël Carré <funman at videolan.org>
> wrote:
>>>> +/**
>>>> + * Initializes a semaphore.
>>>> + */
>>>> +void vlc_sem_init (vlc_sem_t *sem, unsigned value)
>>>
>>> This is not going to interoperate with cancellation.
>>
>> What do you mean?
>>
>> Some of those are mandated to be cancellation points?
> 
> sem_wait() and pthread_rwlock_(rd|wr)lock().


http://pubs.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_09.html#tag_02_09_05_02

Only sem_wait() is mandatory and cancellation is handled.

>>>> +void vlc_cancel (vlc_thread_t thread_id)
>>>> +{
>>>> +    bool self = thread_id == pthread_getspecific(thread_key);
>>>> +
>>>> +    thread_id->killed = true;
>>>> +    if (!thread_id->killable)
>>>> +        return;
>>>> +
>>>> +    vlc_mutex_t *lock = thread_id->lock;
>>>
>>> I fail to grasp where the memory barriers are.
>>
>> Hm i am not sure how barriers work.
>>
>> On android ldr/str is one instruction so it should be atomic?
> 
> I very much doubt that LDR and STR are atomic on ARM SMP.
> 
> Also, nothing warrants that the compiler will emit a single load
> instruction.

I will add __sync_synchronize() before reading.

Maybe we should use atomic instead but support is not in google's gcc 4.4

> 
>>>> +    if (lock) {
>>>> +        if (!self)
>>>> +            vlc_mutex_lock(lock);
>>>
>>> I don't see what warrants that the mutex still exists at this point.
>>
>> vlc_join(thread_id) has not been called yet.
> 
> So the thread handle exists. But my question concerns the mutex.

Well we assume if the thread was waiting on a condition, the condition's
mutex has not been destroyed.

> Also, I do not think self can be true if lock is non-NULL.

You're right.

>>>> +        if (thread_id->cond)
>>>> +            pthread_cond_broadcast(thread_id->cond);
>>>> +        if (!self)
>>>> +            vlc_mutex_unlock(lock);
>>>> +    }
>>>> +
>>>> +    if (self)
>>>> +        vlc_testcancel();
>>>
>>> No, vlc_cancel() is not a cancellation point.
>>
>> In this case, the thread is cancelling itself (can that happen?).
> 
> vlc_cancel() is not a cancellation point in any case.

OK I'll remove and see what happens, I guess the thread reaches another
cancellation point after vlc_cancel(itself).