[vlc-devel] [vlc-commits] objects: fix data-race in debug code

[Steve Lhomme]

Isn't the code supposed to assert on recursive locks ?

PrintObjectPrefix() is called by PrintObject() which is called by 
DumpStructure() which already has the lock.

On 3/13/2019 11:39 AM, Thomas Guillem wrote:
> vlc | branch: master | Thomas Guillem <thomas at gllm.fr> | Wed Mar 13 11:35:01 2019 +0100| [02d494255ce1b8b9c8357cbef5d7eda2a5264495] | committer: Thomas Guillem
>
> objects: fix data-race in debug code
>
> assert(!ObjectHasChild(obj)) was causing the following data_race:
>
> WARNING: ThreadSanitizer: data race (pid=18491)
>    Write of size 8 at 0x7b7000041888 by thread T7 (mutexes: write M150, write M148, write M18):
>      #0 vlc_list_add_between ../../include/vlc_list.h:75 (libvlccore.so.9+0xe4765)
>      #1 vlc_list_add_before ../../include/vlc_list.h:100 (libvlccore.so.9+0xe4765)
>      #2 vlc_list_append ../../include/vlc_list.h:112 (libvlccore.so.9+0xe4765)
>      #3 vlc_custom_create ../../src/misc/objects.c:247 (libvlccore.so.9+0xe4765)
>      #4 CreateDecoder ../../src/input/decoder.c:1765 (libvlccore.so.9+0x69538)
>      #5 decoder_New ../../src/input/decoder.c:2014 (libvlccore.so.9+0x69d1d)
>      #6 input_DecoderNew ../../src/input/decoder.c:2077 (libvlccore.so.9+0x6a7a4)
>      #7 EsOutCreateDecoder ../../src/input/es_out.c:1824 (libvlccore.so.9+0x7025d)
>      #8 EsOutSelectEs ../../src/input/es_out.c:1932 (libvlccore.so.9+0x706c2)
>      #9 EsOutSelect ../../src/input/es_out.c:2164 (libvlccore.so.9+0x716e6)
>      #10 EsOutVaControlLocked ../../src/input/es_out.c:2523 (libvlccore.so.9+0x77d57)
>      #11 EsOutControl ../../src/input/es_out.c:3208 (libvlccore.so.9+0x79f3e)
>      #12 es_out_vaControl ../../include/vlc_es_out.h:158 (libvlccore.so.9+0x7c1ba)
>      #13 es_out_Control ../../include/vlc_es_out.h:167 (libvlccore.so.9+0x7c1ba)
>      #14 CmdExecuteControl ../../src/input/es_out_timeshift.c:1556 (libvlccore.so.9+0x7c4a8)
>      #15 ControlLocked ../../src/input/es_out_timeshift.c:638 (libvlccore.so.9+0x7df40)
>      #16 Control ../../src/input/es_out_timeshift.c:764 (libvlccore.so.9+0x7e35c)
>      #17 es_out_vaControl ../../include/vlc_es_out.h:158 (libvlccore.so.9+0x7fd81)
>      #18 es_out_Control ../../include/vlc_es_out.h:167 (libvlccore.so.9+0x7fd81)
>      #19 es_out_SetMode ../../src/input/es_out.h:97 (libvlccore.so.9+0x82b25)
>      #20 InitPrograms ../../src/input/input.c:1304 (libvlccore.so.9+0x82b25)
>      #21 Init ../../src/input/input.c:1385 (libvlccore.so.9+0x8887c)
>      #22 Run ../../src/input/input.c:532 (libvlccore.so.9+0x89757)
>
>    Previous read of size 8 at 0x7b7000041888 by thread T20:
>      #0 vlc_list_it_next ../../include/vlc_list.h:229 (libvlccore.so.9+0xe42b9)
>      #1 ObjectHasChild ../../src/misc/objects.c:83 (libvlccore.so.9+0xe42b9)
>      #2 vlc_object_release ../../src/misc/objects.c:405 (libvlccore.so.9+0xe4adf)
>      #3 vlc_object_delete ../../include/vlc_objects.h:120 (libvlccore.so.9+0x59210)
>      #4 InvokeModule ../../src/preparser/fetcher.c:161 (libvlccore.so.9+0x59210)
>      #5 SearchArt ../../src/preparser/fetcher.c:188 (libvlccore.so.9+0x5925a)
>      #6 SearchByScope ../../src/preparser/fetcher.c:207 (libvlccore.so.9+0x5a029)
>      #7 SearchNetwork ../../src/preparser/fetcher.c:309 (libvlccore.so.9+0x5a068)
>      #8 FetcherThread ../../src/preparser/fetcher.c:338 (libvlccore.so.9+0x59153)
>
>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=02d494255ce1b8b9c8357cbef5d7eda2a5264495
> ---
>
>   src/misc/objects.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/src/misc/objects.c b/src/misc/objects.c
> index 1f92381c26..07bea99a85 100644
> --- a/src/misc/objects.c
> +++ b/src/misc/objects.c
> @@ -76,7 +76,7 @@ static bool ObjectIsLastChild(vlc_object_t *obj, vlc_object_t *parent)
>       return true;
>   }
>   
> -static bool ObjectHasChild(vlc_object_t *obj)
> +static bool ObjectHasChildLocked(vlc_object_t *obj)
>   {
>       vlc_object_internals_t *priv;
>   
> @@ -85,6 +85,14 @@ static bool ObjectHasChild(vlc_object_t *obj)
>       return false;
>   }
>   
> +static bool ObjectHasChild(vlc_object_t *obj)
> +{
> +    vlc_mutex_lock(&tree_lock);
> +    bool ret = ObjectHasChildLocked(obj);
> +    vlc_mutex_unlock(&tree_lock);
> +    return ret;
> +}
> +
>   static void PrintObjectPrefix(vlc_object_t *obj, bool last)
>   {
>       vlc_object_t *parent = vlc_object_parent(obj);
> @@ -111,7 +119,7 @@ static void PrintObject(vlc_object_t *obj)
>   
>       PrintObjectPrefix(obj, true);
>       printf("\xE2\x94\x80\xE2\x94%c\xE2\x95\xB4%p %s, %u refs\n",
> -           ObjectHasChild(obj) ? 0xAC : 0x80,
> +           ObjectHasChildLocked(obj) ? 0xAC : 0x80,
>              (void *)obj, vlc_object_typename(obj), atomic_load(&priv->refs));
>   
>       vlc_restorecancel (canc);
>
> _______________________________________________
> vlc-commits mailing list
> vlc-commits at videolan.org
> https://mailman.videolan.org/listinfo/vlc-commits