[vlc-devel] [RFC] [NOT FOR MERGE] objres: add mutex support

[Steve Lhomme]

On 2020-01-29 8:54, Thomas Guillem wrote:
> 
> On Wed, Jan 29, 2020, at 08:49, Steve Lhomme wrote:
>> On 2020-01-29 8:37, Thomas Guillem wrote:
>>> Hello,
>>>
>>> I would prefer to avoid using a pointer to access a mutex.
>>>
>>> We will end up with modules using vlc_mutex_lock(sys->lock) and vlc_mutex_lock(&sys->lock).
>>> I think it will add more confusions.
>>
>> How about vlc_obj_mutex_new(vlc_object_t *, vlc_mutex_t *)
> 
> Where is the mutex stored in that case ?
> 
> The only other possibility I could see is:
> 
> vlc_obj_mutex_create(vlc_object_t *, void *objres, vlc_mutex_t *);
> 
> Example:
> struct sys *sys = vlc_obj_malloc(obj, size);
> vlc_obj_mutex_create(obj, sys, &sys->lock);

That could be a possibility, since it's the pattern we always use 
anyway. The code using vlc_obj_mutex_create() most likely already use 
vlc_obj_malloc() for its sys.


> And vlc_obj_mutex_create() need to ensure that the mutex is stored in the objres.
> 
> 
>>
>> The structure storage wouldn't be created (as it is now) and the mutex
>> is accessed the same way as before.
>>
>>> On Tue, Jan 28, 2020, at 21:30, Rémi Denis-Courmont wrote:
>>>> This adds a function to allocate a mutex that automatically gets
>>>> destroyed when a module fails or terminates.
>>>>
>>>> I have very mixed feelings about this, as it introduces an error return
>>>> where there is normally none, i.e., from vlc_mutex_init(). It also
>>>> requires tedious rework to replace `vlc_mutex_t' with `vlc_mutex_t *'.
>>>>
>>>> For reference, kernel code entirely solves this conundrum by not
>>>> requiring explicit mutex destruction (a kernel mutex does not hold any
>>>> resource besides its own `struct mutex' storage space). This could also
>>>> be done, but with significant reworking of the VLC threading layer and
>>>> prevents reliable detection of attempt to delete a still-in-use mutex.
>>>> ---
>>>>    include/vlc_objects.h |  2 ++
>>>>    src/misc/objres.c     | 15 +++++++++++++++
>>>>    2 files changed, 17 insertions(+)
>>>>
>>>> diff --git a/include/vlc_objects.h b/include/vlc_objects.h
>>>> index 19ee5207fc..32f23abf41 100644
>>>> --- a/include/vlc_objects.h
>>>> +++ b/include/vlc_objects.h
>>>> @@ -295,5 +295,7 @@ VLC_API VLC_MALLOC char
>>>> *vlc_obj_strdup(vlc_object_t *obj, const char *str);
>>>>     */
>>>>    VLC_API void vlc_obj_free(vlc_object_t *obj, void *ptr);
>>>>    
>>>> +VLC_API VLC_MALLOC vlc_mutex_t *vlc_obj_mutex_new(vlc_object_t *obj);
>>>> +
>>>>    /** @} */
>>>>    /** @} */
>>>> diff --git a/src/misc/objres.c b/src/misc/objres.c
>>>> index cac8624e7e..383dd760a8 100644
>>>> --- a/src/misc/objres.c
>>>> +++ b/src/misc/objres.c
>>>> @@ -174,3 +174,18 @@ void vlc_obj_free(vlc_object_t *obj, void *ptr)
>>>>    {
>>>>        vlc_objres_remove(obj, ptr, ptrcmp);
>>>>    }
>>>> +
>>>> +static void mutex_release(void *data)
>>>> +{
>>>> +    vlc_mutex_destroy(data);
>>>> +}
>>>> +
>>>> +vlc_mutex_t *vlc_obj_mutex_new(vlc_object_t *obj)
>>>> +{
>>>> +    vlc_mutex_t *mtx = vlc_objres_new(sizeof (*mtx), mutex_release);
>>>> +    if (mtx != NULL) {
>>>> +        vlc_mutex_init(mtx);
>>>> +        vlc_objres_push(obj, mtx);
>>>> +    }
>>>> +    return mtx;
>>>> +}
>>>> -- 
>>>> 2.25.0
>>>>
>>>> _______________________________________________
>>>> vlc-devel mailing list
>>>> To unsubscribe or modify your subscription options:
>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>