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

[Thomas Guillem]

On Wed, Mar 13, 2019, at 12:24, Steve Lhomme wrote:
> 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.

Yes that is why it call ObjectHasChildLocked().

ObjectHasChild() is only called from  assert(!ObjectHasChild(obj))

> 
> 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
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel