<!DOCTYPE html><html><head><title></title><style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style></head><body><div><br></div><div>On Thu, Feb 21, 2019, at 11:18, Rémi Denis-Courmont wrote:<br></div><blockquote type="cite" id="fastmail-quoted"><div>Hi,<br></div><div><br></div><div>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.<br></div></blockquote><div><br></div><div>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...<br></div><div><br></div><blockquote type="cite" id="fastmail-quoted"><div><br></div><div>Not only that but stderr is not universally the output for debug nor is it universally where assertion failures are dumped to, <br></div></blockquote><div>So if there is no stderr, you don't see the error. I don't see the problem at all.<br></div><blockquote type="cite" id="fastmail-quoted"><div>so this patch does not even work properly<br></div></blockquote><div>Works on Windows, Android, Linux, macOS when VLC is run via command line. This is the case for most developers usecase.<br></div><div><br></div><blockquote type="cite" id="fastmail-quoted"><div><br></div><div>And in most cases you can see the leak by stopping all inputs and dumping the objects tree before quitting, anyway.<br></div></blockquote><div><br></div><div>What about leaks not related to playback ? Interface, service discovery, playlist that are only cleaned when vlc is closed.<br></div><div><br></div><blockquote type="cite" id="fastmail-quoted"><div><br></div><div class="fastmail-quoted-gmail_quote"><div>Le 21 février 2019 11:24:57 GMT+02:00, Thomas Guillem <thomas@gllm.fr> a écrit :<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:0pt;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="fastmail-quoted-gmail_quote"><pre class="fastmail-quoted-k9mail"><div>Comment:<br></div><div>A lot of developers asked me to propose this patch again. It was previously<br></div><div>refused because developers could use proper debugging tools. I disagree with<br></div><div>that assumption. Even with Asan, when the leak assert fail, it will abort()<br></div><div>without prompting any leaks, so you'll have to edit libvlc.c to remove this<br></div><div>assert. If your leak was not reproducible, you'll lose a precious information.<br></div><div>And good luck finding the leak with GDB, you really need to know the internal<br></div><div>of vlc_object_t to iterate through the different list to get the culprit.<br></div><div><br></div><div>Commit log:<br></div><div><br></div><div>In case of a vlc_object_t leak, and if asserts are enabled, the error output<br></div><div>will be like the following (when leaking intentionally decoder_t objects):<br></div><div><br></div><div>=== vlc_object LEAKS detected ===<br></div><div>  \ playlist<br></div><div>  |   \ input<br></div><div>  |   |   \ decoder avcodec<br></div><div>  |   |   | decoder faad<br></div><div>  |   |   | decoder stl<hr> src/libvlc.c | 30 ++++++++++++++++++++++++++++++<br></div><div> 1 file changed, 30 insertions(+)<br></div><div><br></div><div>diff --git a/src/libvlc.c b/src/libvlc.c<br></div><div>index 8e8d2ca8b2..d9ff96de84 100644<br></div><div>--- a/src/libvlc.c<br></div><div>+++ b/src/libvlc.c<br></div><div>@@ -436,6 +436,24 @@ void libvlc_InternalCleanup( libvlc_int_t *p_libvlc )<br></div><div> #endif<br></div><div> }<br></div><div> <br></div><div>+#ifndef NDEBUG<br></div><div>+static void DumpObjectLeaks(vlc_object_internals_t *priv, unsigned level)<br></div><div>+{<br></div><div>+    bool first = true;<br></div><div>+    vlc_list_foreach(priv, &priv->children, siblings)<br></div><div>+    {<br></div><div>+        vlc_object_t *obj = vlc_externals(priv);<br></div><div>+        for (unsigned i = 0; i < level; i++)<br></div><div>+            fprintf(stderr, "  %s ", first && i == level -1 ? "\\" : "|");<br></div><div>+        char *name = vlc_object_get_name(obj);<br></div><div>+        fprintf(stderr, "%s %s\n", vlc_object_typename(obj), name ? name : "");<br></div><div>+        free(name);<br></div><div>+        DumpObjectLeaks(priv, level + 1);<br></div><div>+        first = false;<br></div><div>+    }<br></div><div>+}<br></div><div>+#endif<br></div><div>+<br></div><div> /**<br></div><div>  * Destroy everything.<br></div><div>  * This function requests the running threads to finish, waits for their<br></div><div>@@ -449,6 +467,18 @@ void libvlc_InternalDestroy( libvlc_int_t *p_libvlc )<br></div><div> <br></div><div>     vlc_ExitDestroy( &priv->exit );<br></div><div> <br></div><div>+#ifndef NDEBUG<br></div><div>+    {<br></div><div>+        vlc_object_internals_t *internal = vlc_internals(p_libvlc);<br></div><div>+        if (atomic_load(&internal->refs) != 1)<br></div><div>+        {<br></div><div>+            vlc_mutex_lock(&internal->tree_lock);<br></div><div>+            fprintf(stderr, "=== vlc_object LEAKS detected ===\n");<br></div><div>+            DumpObjectLeaks(internal, 1);<br></div><div>+            vlc_mutex_unlock(&internal->tree_lock);<br></div><div>+        }<br></div><div>+    }<br></div><div>+#endif<br></div><div>     assert( atomic_load(&(vlc_internals(p_libvlc)->refs)) == 1 );<br></div><div>     vlc_object_release( p_libvlc );<br></div><div> }<br></div></pre></blockquote></div><div><br></div><div>-- <br></div><div>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté. <br></div><div>_______________________________________________<br></div><div>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div>https://mailman.videolan.org/listinfo/vlc-devel<br></div></blockquote><div><br></div></body></html>