After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 767862 - leaks tracer: display creation stack trace of leaked objects
leaks tracer: display creation stack trace of leaked objects
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: 1.9.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 767857
Blocks:
 
 
Reported: 2016-06-20 09:41 UTC by Guillaume Desmottes
Modified: 2016-07-08 09:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
leaks: add creation stack trace support (9.00 KB, patch)
2016-06-23 15:47 UTC, Guillaume Desmottes
none Details | Review
leaks: add creation stack trace support (9.22 KB, patch)
2016-06-24 09:14 UTC, Guillaume Desmottes
none Details | Review
leaks: add creation stack trace support (9.39 KB, patch)
2016-07-08 08:59 UTC, Guillaume Desmottes
committed Details | Review

Description Guillaume Desmottes 2016-06-20 09:41:28 UTC
As suggested on https://bugzilla.gnome.org/show_bug.cgi?id=765052#c62 it would be nice to be able to collect a stack trace when objects are created so we can more easily track leaked objects.

Options to do so include:
- backtrace() from libc
- http://www.nongnu.org/libunwind/ as an optional dep to enable this feature.
Comment 1 Guillaume Desmottes 2016-06-20 13:24:19 UTC
Here is the kind of trace with backtrace():

/home/cassidy/gst/master/gstreamer/plugins/tracers/.libs/libgstcoretracers.so(+0x3fe8) [0x7f1675477fe8]
/home/cassidy/gst/master/gstreamer/plugins/tracers/.libs/libgstcoretracers.so(+0x412b) [0x7f167547812b]
/home/cassidy/gst/master/gstreamer/gst/.libs/libgstreamer-1.0.so.0(+0x2d75f) [0x7f167d7fb75f]
/lib64/libgobject-2.0.so.0(+0x150f7) [0x7f167d28f0f7]
/lib64/libgobject-2.0.so.0(g_object_newv+0xd1) [0x7f167d290461]
/home/cassidy/gst/master/gstreamer/gst/.libs/libgstreamer-1.0.so.0(+0xa1dfa) [0x7f167d86fdfa]
/home/cassidy/gst/master/gstreamer/gst/.libs/libgstreamer-1.0.so.0(gst_deinit+0xf2) [0x7f167d7fac42]
/home/cassidy/gst/master/gstreamer/tools/.libs/lt-gst-launch-1.0() [0x403899]
/lib64/libc.so.6(__libc_start_main+0xf0) [0x7f167c372580]
/home/cassidy/gst/master/gstreamer/tools/.libs/lt-gst-launch-1.0() [0x403ee9]

and the ones, much better, with libunwind:

handle_object_created.part.0
gst_object_constructed
g_object_unref
g_object_newv
init_klass_pool.isra.0
gst_deinit
main
__libc_start_main
_start

I'll use this one with an optional dep on libunwind then.
Comment 2 Stefan Sauer (gstreamer, gtkdoc dev) 2016-06-20 13:29:00 UTC
+1 Ideally we do a add a convenience api to get a backtrace and then use libunwind if present and fallback to backtrace() and backtrace_symbols() otherwise.
FYI. The not so nice glibc backtrace can be showed through addr2line to make it pretty, but shelling out to a commandline tool is meh :/
Comment 3 Guillaume Desmottes 2016-06-23 15:47:55 UTC
Created attachment 330270 [details] [review]
leaks: add creation stack trace support

This allow us to provide the trace of leaked objects making it easier
to debug.
Comment 4 Guillaume Desmottes 2016-06-23 15:48:52 UTC
This patch depends of the ones fixing bug #767857
Comment 5 Sebastian Dröge (slomo) 2016-06-24 08:22:15 UTC
Review of attachment 330270 [details] [review]:

::: configure.ac
@@ +810,3 @@
+
+if test "x$HAVE_UNWIND" = "xyes"; then
+AC_DEFINE(HAVE_UNWIND, 1, [libunwind available])

Indentation

::: plugins/tracers/gstleaks.c
@@ +34,3 @@
 
+#ifdef HAVE_UNWIND
+#define UNW_LOCAL_ONLY

What does this do?

@@ +194,3 @@
+    name[0] = '\0';
+    if (unw_get_proc_name (&cursor, name, 256, &offp) != 0)
+      break;

This might be cut off here and the last byte might not be '\0', right? In which case you need to make it that.
Comment 6 Guillaume Desmottes 2016-06-24 09:14:37 UTC
Review of attachment 330270 [details] [review]:

::: configure.ac
@@ +810,3 @@
+
+if test "x$HAVE_UNWIND" = "xyes"; then
+  AC_CHECK_HEADERS([execinfo.h], [

fixed.

::: plugins/tracers/gstleaks.c
@@ +34,3 @@
 
+#ifdef HAVE_UNWIND
+#define UNW_LOCAL_ONLY

added a comment.

@@ +194,3 @@
+    name[0] = '\0';
+    if (unw_get_proc_name (&cursor, name, 256, &offp) != 0)
+  trace = g_string_new (NULL);

Just checked and the buffer is always \0 terminated even when truncated.
Comment 7 Guillaume Desmottes 2016-06-24 09:14:47 UTC
Created attachment 330304 [details] [review]
leaks: add creation stack trace support

This allow us to provide the trace of leaked objects making it easier
to debug.
Comment 8 Sebastian Dröge (slomo) 2016-07-08 08:28:22 UTC
Review of attachment 330304 [details] [review]:

::: plugins/tracers/gstleaks.c
@@ +241,3 @@
+#ifdef HAVE_BACKTRACE
+  return generate_backtrace_trace ();
+#endif /* HAVE_BACKTRACE */

What if we have both? Then we have unreachable code here and compile both in :)

@@ +356,3 @@
   leak->ref_count = ref_count;
   leak->desc = gst_info_strdup_printf ("%" GST_PTR_FORMAT, obj);
+  leak->trace = trace;

Is this guaranteed to be a valid pointer long enough, or should it be strdup()?
Comment 9 Guillaume Desmottes 2016-07-08 08:59:24 UTC
Review of attachment 330304 [details] [review]:

::: plugins/tracers/gstleaks.c
@@ +241,3 @@
+#ifdef HAVE_BACKTRACE
+  return generate_backtrace_trace ();
+#define BT_BUF_SIZE 100

Fixed this by still using the fallback if, for some reason, libunwind failed.

@@ +356,3 @@
   leak->ref_count = ref_count;
   leak->desc = gst_info_strdup_printf ("%" GST_PTR_FORMAT, obj);
+  leak->trace = trace;

Yep, added a comment.
Comment 10 Guillaume Desmottes 2016-07-08 08:59:34 UTC
Created attachment 331058 [details] [review]
leaks: add creation stack trace support

This allow us to provide the trace of leaked objects making it easier
to debug.
Comment 11 Sebastian Dröge (slomo) 2016-07-08 09:10:18 UTC
commit 17c37efa83cfd356aa2fa8d9b1c8959251cfb3ba
Author: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date:   Wed Jun 22 16:25:16 2016 +0200

    leaks tracer: add creation stack trace support
    
    This allow us to provide the trace of leaked objects making it easier
    to debug.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=767862