[vlc-devel] [PATCH 1/6] os2: implement vlc_once()

KO Myung-Hun komh78 at gmail.com
Tue Apr 3 15:14:57 CEST 2018



Rémi Denis-Courmont wrote:
> Le 3 avril 2018 13:07:50 GMT+02:00, Kamil Rytarowski <n54 at gmx.com> a écrit :
>> On 03.04.2018 12:48, Rémi Denis-Courmont wrote:
>>> Le 3 avril 2018 12:35:15 GMT+02:00, Kamil Rytarowski <n54 at gmx.com> a
>> écrit :
>>>> On 03.04.2018 08:39, Rémi Denis-Courmont wrote:
>>>>> Le mardi 3 avril 2018, 00:12:53 EEST Kamil Rytarowski a écrit :
>>>>>> On 23.03.2018 09:54, Rémi Denis-Courmont wrote:
>>>>>>> Le jeudi 22 mars 2018, 16:41:29 EET KO Myung-Hun a écrit :
>>>>>>>> ---
>>>>>>>>
>>>>>>>>  include/vlc_threads.h |  6 ++++++
>>>>>>>>  src/os2/thread.c      | 17 +++++++++++++++++
>>>>>>>>  2 files changed, 23 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/include/vlc_threads.h b/include/vlc_threads.h
>>>>>>>> index 37054a3ded..b7505da266 100644
>>>>>>>> --- a/include/vlc_threads.h
>>>>>>>> +++ b/include/vlc_threads.h
>>>>>>>> @@ -125,6 +125,12 @@ typedef struct
>>>>>>>>
>>>>>>>>  #define VLC_STATIC_COND { NULLHANDLE, 0, NULLHANDLE, 0 }
>>>>>>>>  #define LIBVLC_NEED_SEMAPHORE
>>>>>>>>  #define LIBVLC_NEED_RWLOCK
>>>>>>>>
>>>>>>>> +typedef struct
>>>>>>>> +{
>>>>>>>> +    unsigned done;
>>>>>>>> +    vlc_mutex_t mutex;
>>>>>>>> +} vlc_once_t;
>>>>>>>> +#define VLC_STATIC_ONCE { 0, VLC_STATIC_MUTEX }
>>>>>>>>
>>>>>>>>  typedef struct vlc_threadvar *vlc_threadvar_t;
>>>>>>>>  typedef struct vlc_timer *vlc_timer_t;
>>>>>>>>
>>>>>>>> diff --git a/src/os2/thread.c b/src/os2/thread.c
>>>>>>>> index 8f7b335ea4..52f58023b8 100644
>>>>>>>> --- a/src/os2/thread.c
>>>>>>>> +++ b/src/os2/thread.c
>>>>>>>> @@ -427,6 +427,23 @@ int vlc_cond_timedwait_daytime (vlc_cond_t
>>>>>>>> *p_condvar,
>>>>>>>> vlc_mutex_t *p_mutex, return vlc_cond_wait_common (p_condvar,
>>>> p_mutex,
>>>>>>>> ulTimeout);
>>>>>>>>
>>>>>>>>  }
>>>>>>>>
>>>>>>>> +void vlc_once(vlc_once_t *once, void (*cb)(void))
>>>>>>>> +{
>>>>>>>> +    if( !once->done )
>>>>>>>
>>>>>>> This looks like a violation of the memory model.
>>>>>>
>>>>>> This patch is correct.
>>>>>
>>>>> No, it is not.
>>>>>
>>>>
>>>> Show a testcase breaking it with correct usage of this API,
>> otherwise
>>>> this claim is to be treated as unreasonable and shall be ignored,
>> and
>>>> patch merged.
>>>
>>> Such a process could work in a fully defined language, such as single
>> threaded Java. C is not a fully defined language in general and
>> multithreaded C in particular.
>>>
>>> For such a small function, it is pretty damn obvious that access to
>> pto_done is not according to either types of accesses defined by the
>> POSIX memory model. It only works if there is no race, but the whole
>> point of the function is to handle potential races.
>>>
>>
>> A testcase is still missing.
> 
> Call the function on one thread. Then when it has taken the mutex, call the function on another thread with the same one-time initiliazer.
> 
> pto_done gets written on one thread and read on another one. UB. QED.
> 

Is this problem ?

If pto_done gets written, it means that init routine has been called
already. If it is read on other thread after written, then vlc_once()
will return soon with completing init routine already. Wrong ?

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v4.1.32 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.os2.kr/



More information about the vlc-devel mailing list