<html><head></head><body>Hi,<br><br>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.<br><br>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.<br><br><div class="gmail_quote">Le 21 février 2019 12:26:56 GMT+02:00, Alexandre Janniaux <ajanni@videolabs.io> 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">Hi,<br><br>I think that not being universally correct is not a problem for a<br>debugging tool. I find this patch quite useful, especially for windows<br>and beginner developer in the project, and would be a good addition to<br>ASan debugging.<br><br>If universality is a blocking issue, it could be activated with a<br>--with-sanitizer=vlcobject like parameter.<br><br>On 2019-02-21 11:18, 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>Well I did debug that assertion failure using (ASan +) gdb just fine<br>last time. The code has not changed. So this kludge is definitely not<br>necessary for its alleged purpose.<br><br>Not only that but stderr is not universally the output for debug nor<br>is it universally where assertion failures are dumped to, so this<br>patch does not even work properly<br><br>And in most cases you can see the leak by stopping all inputs and<br>dumping the objects tree before quitting, anyway.<br><br>Le 21 février 2019 11:24:57 GMT+02:00, Thomas Guillem <thomas@gllm.fr> <br>a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">Comment:<br>A lot of developers asked me to propose this patch again. It was<br>previously<br>refused because developers could use proper debugging tools. I <br>disagree<br>with<br>that assumption. Even with Asan, when the leak assert fail, it will<br>abort()<br>without prompting any leaks, so you'll have to edit libvlc.c to remove<br>this<br>assert. If your leak was not reproducible, you'll lose a precious<br>information.<br>And good luck finding the leak with GDB, you really need to know the<br>internal<br>of vlc_object_t to iterate through the different list to get the<br>culprit.<br><br>Commit log:<br><br>In case of a vlc_object_t leak, and if asserts are enabled, the error<br>output<br>will be like the following (when leaking intentionally decoder_t<br>objects):<br><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><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( libvlc_int_t<br>*p_libvlc )<br>#endif<br>}<br><br>+#ifndef NDEBUG<br>+static void DumpObjectLeaks(vlc_object_internals_t *priv, unsigned<br>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>"|");<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 for <br>their<br>@@ -449,6 +467,18 @@ void libvlc_InternalDestroy( libvlc_int_t<br>*p_libvlc )<br><br> vlc_ExitDestroy( &priv->exit );<br><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>2.20.1<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><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>