[vlc-devel] [PATCH] libvlc: dump object types abd names when leaking

[Rémi Denis-Courmont]

Hi,

I think that being worse than existing tools is a problem for a debugging tool. This is a completely ad-hoc kludge that solves a more specific problem (one specific assertion) and in a worse way (less data available) than existing tools.

I cannot deny that printf debugging is easy to use - its main benefit. But I don't clobber the code base with my printf debugging, at least not consciously.

Le 21 février 2019 12:26:56 GMT+02:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
>Hi,
>
>I think that not being universally correct is not a problem for a
>debugging tool. I find this patch quite useful, especially for windows
>and beginner developer in the project, and would be a good addition to
>ASan debugging.
>
>If universality is a blocking issue, it could be activated with a
>--with-sanitizer=vlcobject like parameter.
>
>On 2019-02-21 11:18, Rémi Denis-Courmont wrote:
>> Hi,
>> 
>> Well I did debug that assertion failure using (ASan +) gdb just fine
>> last time. The code has not changed. So this kludge is definitely not
>> necessary for its alleged purpose.
>> 
>> Not only that but stderr is not universally the output for debug nor
>> is it universally where assertion failures are dumped to, so this
>> patch does not even work properly
>> 
>> And in most cases you can see the leak by stopping all inputs and
>> dumping the objects tree before quitting, anyway.
>> 
>> Le 21 février 2019 11:24:57 GMT+02:00, Thomas Guillem
><thomas at gllm.fr> 
>> a écrit :
>>> Comment:
>>> A lot of developers asked me to propose this patch again. It was
>>> previously
>>> refused because developers could use proper debugging tools. I 
>>> disagree
>>> with
>>> that assumption. Even with Asan, when the leak assert fail, it will
>>> abort()
>>> without prompting any leaks, so you'll have to edit libvlc.c to
>remove
>>> this
>>> assert. If your leak was not reproducible, you'll lose a precious
>>> information.
>>> And good luck finding the leak with GDB, you really need to know the
>>> internal
>>> of vlc_object_t to iterate through the different list to get the
>>> culprit.
>>> 
>>> Commit log:
>>> 
>>> In case of a vlc_object_t leak, and if asserts are enabled, the
>error
>>> output
>>> will be like the following (when leaking intentionally decoder_t
>>> objects):
>>> 
>>> === vlc_object LEAKS detected ===
>>>  \ playlist
>>>  |   \ input
>>>  |   |   \ decoder avcodec
>>>  |   |   | decoder faad
>>>  |   |   | decoder stl
>>> ---
>>> src/libvlc.c | 30 ++++++++++++++++++++++++++++++
>>> 1 file changed, 30 insertions(+)
>>> 
>>> diff --git a/src/libvlc.c b/src/libvlc.c
>>> index 8e8d2ca8b2..d9ff96de84 100644
>>> --- a/src/libvlc.c
>>> +++ b/src/libvlc.c
>>> @@ -436,6 +436,24 @@ void libvlc_InternalCleanup( libvlc_int_t
>>> *p_libvlc )
>>> #endif
>>> }
>>> 
>>> +#ifndef NDEBUG
>>> +static void DumpObjectLeaks(vlc_object_internals_t *priv, unsigned
>>> level)
>>> +{
>>> +    bool first = true;
>>> +    vlc_list_foreach(priv, &priv->children, siblings)
>>> +    {
>>> +        vlc_object_t *obj = vlc_externals(priv);
>>> +        for (unsigned i = 0; i < level; i++)
>>> +            fprintf(stderr, "  %s ", first && i == level -1 ? "\\"
>:
>>> "|");
>>> +        char *name = vlc_object_get_name(obj);
>>> +        fprintf(stderr, "%s %s\n", vlc_object_typename(obj), name ?
>>> name : "");
>>> +        free(name);
>>> +        DumpObjectLeaks(priv, level + 1);
>>> +        first = false;
>>> +    }
>>> +}
>>> +#endif
>>> +
>>> /**
>>>  * Destroy everything.
>>> * This function requests the running threads to finish, waits for 
>>> their
>>> @@ -449,6 +467,18 @@ void libvlc_InternalDestroy( libvlc_int_t
>>> *p_libvlc )
>>> 
>>>     vlc_ExitDestroy( &priv->exit );
>>> 
>>> +#ifndef NDEBUG
>>> +    {
>>> +        vlc_object_internals_t *internal = vlc_internals(p_libvlc);
>>> +        if (atomic_load(&internal->refs) != 1)
>>> +        {
>>> +            vlc_mutex_lock(&internal->tree_lock);
>>> +            fprintf(stderr, "=== vlc_object LEAKS detected ===\n");
>>> +            DumpObjectLeaks(internal, 1);
>>> +            vlc_mutex_unlock(&internal->tree_lock);
>>> +        }
>>> +    }
>>> +#endif
>>>     assert( atomic_load(&(vlc_internals(p_libvlc)->refs)) == 1 );
>>>     vlc_object_release( p_libvlc );
>>> }
>>> --
>>> 2.20.1
>>> 
>>> _______________________________________________
>>> 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

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190221/333ef9a0/attachment.html>