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

Rémi Denis-Courmont remi at remlab.net
Thu Feb 21 12:49:33 CET 2019


So... You say that your debugging environment sucks.

That is an argument for you to fix your debugging environment, or use another environment.

That is not an argument for adding printf debugging to one specific assertion out of many, which does not solve your problem.

Le 21 février 2019 13:32:04 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>On 21/02/2019 11:58, Rémi Denis-Courmont wrote:
>> It's making a special snow flake of that one specific assertion.
>>
>> If you want to develop tooling for dumping the object tree, you're 
>> much better off scripting your favorite debugger than cluttering one 
>> specific VLC assertion.
>
>I don't think that's possible in QtCreator (and that's not my favorite 
>debugger by far), while writing a few lines of code in C is very easy.
>
>On Windows gdb doesn't break on asserts. It could be enabled to do it, 
>but then everytime I'd run VLC under gdb it breaks 20 times in weird Qt
>
>places. So I always end up disabling it. So when a leak occurs (or any 
>assert for that matter) I have to go through this and pray I'll have
>the 
>leak/assert again.
>
>>
>> Le 21 février 2019 12:44:44 GMT+02:00, Steve Lhomme
><robux4 at ycbcr.xyz> 
>> a écrit :
>>
>>     On 21/02/2019 11:37, Rémi Denis-Courmont wrote:
>>
>>         Hi, My GDB skills are bad so I just walked the objects tree
>by
>>         printing memory content on the abort. It would be much easier
>>         with VLC-specific GDB macros, but the point is that you can
>>         debug that assertion just as any. It also works after the
>fact
>>         for pseudorandom failures, if you enable core dumps. Much
>>         better than this patch since you have access to all memory,
>>         not just the objects tree. Adding special snow flake debug
>>         code for assertion is a bad idea that sets a very bad
>>         precedent - as opposed to improving integration with proper
>>         debugging tools. 
>>
>>
>>     What's snow flake about adding debug tooling ?
>>
>>     >
>>
>>         Le 21 février 2019 12:26:47 GMT+02:00, Thomas Guillem
>>         <thomas at gllm.fr> a écrit : On Thu, Feb 21, 2019, at 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. 
>>
>>         Even if it works with gdb (please tell us how), evelopers
>>         don't always run VLC with gdb. This cause an issue if the
>leak
>>         is not reproducible...
>>
>>             Not only that but stderr is not universally the output
>for
>>             debug nor is it universally where assertion failures are
>>             dumped to, 
>>
>>         So if there is no stderr, you don't see the error. I don't
>see
>>         the problem at all.
>>
>>             so this patch does not even work properly 
>>
>>         Works on Windows, Android, Linux, macOS when VLC is run via
>>         command line. This is the case for most developers usecase.
>>
>>             And in most cases you can see the leak by stopping all
>>             inputs and dumping the objects tree before quitting,
>anyway. 
>>
>>         What about leaks not related to playback ? Interface, service
>>         discovery, playlist that are only cleaned when vlc is closed.
>>
>>             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 ); } -- Envoyé de mon
>>             appareil Android avec Courriel K-9 Mail. Veuillez excuser
>>             ma brièveté.
>>            
>------------------------------------------------------------------------
>>             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é.
>>        
>------------------------------------------------------------------------
>>         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é.
>>
>> _______________________________________________
>> 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/06e8dff8/attachment.html>


More information about the vlc-devel mailing list