<html><head></head><body>It's making a special snow flake of that one specific assertion.<br><br>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.<br><br><div class="gmail_quote">Le 21 février 2019 12:44:44 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">On 21/02/2019 11:37, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"> Hi,<br><br> My GDB skills are bad so I just walked the objects tree by printing <br> memory content on the abort. It would be much easier with VLC-specific <br> GDB macros, but the point is that you can debug that assertion just as <br> any.<br><br> It also works after the fact for pseudorandom failures, if you enable <br> core dumps. Much better than this patch since you have access to all <br> memory, not just the objects tree.<br><br> Adding special snow flake debug code for assertion is a bad idea that <br> sets a very bad precedent - as opposed to improving integration with <br> proper debugging tools.<br></blockquote><br>What's snow flake about adding debug tooling ?<br><br>><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"> Le 21 février 2019 12:26:47 GMT+02:00, Thomas Guillem <thomas@gllm.fr> <br> a écrit :<br><br><br> On Thu, Feb 21, 2019, at 11:18, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> Hi,<br><br> Well I did debug that assertion failure using (ASan +) gdb just<br> fine last time. The code has not changed. So this kludge is<br> definitely not necessary for its alleged purpose.<br></blockquote> Even if it works with gdb (please tell us how), evelopers don't<br> always run VLC with gdb. This cause an issue if the leak is not<br> reproducible...<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">Not only that but stderr is not universally the output for debug<br>nor is it universally where assertion failures are dumped to,<br></blockquote>So if there is no stderr, you don't see the error. I don't see the<br>problem at all.<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">so this patch does not even work properly<br></blockquote> Works on Windows, Android, Linux, macOS when VLC is run via<br> command line. This is the case for most developers usecase.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">And in most cases you can see the leak by stopping all inputs and<br>dumping the objects tree before quitting, anyway.<br></blockquote> What about leaks not related to playback ? Interface, service<br> discovery, playlist that are only cleaned when vlc is closed.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> Le 21 février 2019 11:24:57 GMT+02:00, Thomas Guillem<br> <thomas@gllm.fr> a écrit :<br><br> Comment:<br> A lot of developers asked me to propose this patch again. It<br> was previously<br> refused because developers could use proper debugging tools.<br> I disagree with<br> that assumption. Even with Asan, when the leak assert fail,<br> it will abort()<br> without prompting any leaks, so you'll have to edit libvlc.c<br> to remove this<br> assert. If your leak was not reproducible, you'll lose a<br> precious information.<br> And good luck finding the leak with GDB, you really need to<br> know the internal<br> of vlc_object_t to iterate through the different list to get<br> the culprit.<br> Commit log:<br> In case of a vlc_object_t leak, and if asserts are enabled,<br> the error output<br> will be like the following (when leaking intentionally<br> decoder_t objects):<br> === vlc_object LEAKS detected ===<br> \ playlist<br> | \ input<br> | | \ decoder avcodec<br> | | | decoder faad<br> | | | decoder stl<hr> src/libvlc.c | 30 ++++++++++++++++++++++++++++++<br> 1 file changed, 30 insertions(+)<br> diff --git a/src/libvlc.c b/src/libvlc.c<br> index 8e8d2ca8b2..d9ff96de84 100644<br> --- a/src/libvlc.c<br> +++ b/src/libvlc.c<br> @@ -436,6 +436,24 @@ void libvlc_InternalCleanup(<br> libvlc_int_t *p_libvlc )<br> #endif<br> }<br> +#ifndef NDEBUG<br> +static void DumpObjectLeaks(vlc_object_internals_t *priv,<br> unsigned level)<br> +{<br> + bool first = true;<br> + vlc_list_foreach(priv, &priv->children, siblings)<br> + {<br> + vlc_object_t *obj = vlc_externals(priv);<br> + for (unsigned i = 0; i < level; i++)<br> + fprintf(stderr, " %s ", first && i == level -1 ? "\\" : "|");<br> + char *name = vlc_object_get_name(obj);<br> + fprintf(stderr, "%s %s\n", vlc_object_typename(obj), name ?<br> name : "");<br> + free(name);<br> + DumpObjectLeaks(priv, level + 1);<br> + first = false;<br> + }<br> +}<br> +#endif<br> +<br> /**<br> * Destroy everything.<br> * This function requests the running threads to finish, waits<br> for their<br> @@ -449,6 +467,18 @@ void libvlc_InternalDestroy(<br> libvlc_int_t *p_libvlc )<br> vlc_ExitDestroy( &priv->exit );<br> +#ifndef NDEBUG<br> + {<br> + vlc_object_internals_t *internal = vlc_internals(p_libvlc);<br> + if (atomic_load(&internal->refs) != 1)<br> + {<br> + vlc_mutex_lock(&internal->tree_lock);<br> + fprintf(stderr, "=== vlc_object LEAKS detected ===\n");<br> + DumpObjectLeaks(internal, 1);<br> + vlc_mutex_unlock(&internal->tree_lock);<br> + }<br> + }<br> +#endif<br> assert( atomic_load(&(vlc_internals(p_libvlc)->refs)) == 1 );<br> vlc_object_release( p_libvlc );<br> }<br><br><br> -- <br> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez<br> excuser ma brièveté.<hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote><br> -- <br> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez <br> excuser ma brièveté.<hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>