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

[Rafaël Carré]

Hello,

Le 05/10/2012 13:28, Rémi Denis-Courmont a écrit :
> On Fri, 05 Oct 2012 12:44:00 +0200, Rafaël Carré <funman at videolan.org>
> wrote:
>> 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.
> 
> It is handled in a very energy-inefficient manner, compared to the
> existing VLC implementation.
> 
> And R/W locking may be a cancellation point really means R/W locking ought
> to be a cancellation point in the slow/contention path.

OK, now using libvlc rwlocks and semaphores.

>>>>>> +    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.
> 
> Nothing proves that the thread is waiting on the condition variable. All
> you know, at the time lock is fetched, the thread has not yet reset lock to
> NULL. That does not mean the thread is still waiting. That definitely does
> not mean the thread will still be waiting a few cycles later when
> pthread_cond_broadcast() gets invoked.
> 
> I cannot see any warranty that lock and cond pointers are valid.
> 
> Also, lock pointer seems useless in this business.
> 

"""
if predictable  scheduling behavior is required, then that mutex shall
be locked   by   the   thread   calling    pthread_cond_broadcast()
or pthread_cond_signal().
"""

I guess it is linked to thread priorities but since they are not handled
in android we can remove it and only fetch the cond:
http://stackoverflow.com/questions/1003190/not-locking-mutex-for-pthread-cond-timedwait-and-pthread-cond-signal-on-linux

cond pointer is valid since the thread has not been joined yet, the
storage is still allocated.

If cond pointer is set, the thread is either waiting on the condition,
or it just woke up.

"""
The pthread_cond_broadcast() and pthread_cond_signal() functions shall
have no effect if there are no threads currently blocked on cond.
"""


Updated patch attached.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-android-threads-support.patch
Type: text/x-patch
Size: 26342 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20121005/f6ea76e3/attachment.bin>