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 790042 - waylandsink: memory leak of shm allocator
waylandsink: memory leak of shm allocator
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.12.2
Other Linux
: Normal normal
: 1.12.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-11-08 02:33 UTC by Shinya Saito
Modified: 2017-12-07 01:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patche that can solve leaks (3.10 KB, patch)
2017-11-08 02:33 UTC, Shinya Saito
committed Details | Review

Description Shinya Saito 2017-11-08 02:33:09 UTC
Created attachment 363186 [details] [review]
Patche that can solve leaks

I tested the next pipelines with valgrind and the following leak was detected in either case.
・valgrind --leak-check=full --suppressions=./gst.supp gst-launch-1.0 videotestsrc ! video/x-raw, format=BGRA ! waylandsink

==3257== 216 (184 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 2,780 of 2,924
==3257==    at 0x49CDB10: g_type_create_instance (gtype.c:1844)
==3257==    by 0x49AE9D3: g_object_new_internal (gobject.c:1783)
==3257==    by 0x49B0647: g_object_newv (gobject.c:1930)
==3257==    by 0x49B0C8B: g_object_new (gobject.c:1623)
==3257==    by 0x7297E93: gst_wl_shm_allocator_register (wlshmallocator.c:114)
==3257==    by 0x7297A7B: plugin_init (gstwaylandsink.c:950)
==3257==    by 0x48F95AB: gst_plugin_register_func (gstplugin.c:524)
==3257==    by 0x48FB3FB: _priv_gst_plugin_load_file_for_registry (gstplugin.c:827)
==3257==    by 0x48FC09B: gst_plugin_load_by_name (gstplugin.c:1266)
==3257==    by 0x48FCB2F: gst_plugin_feature_load (gstpluginfeature.c:112)
==3257==    by 0x48D1707: gst_element_factory_create (gstelementfactory.c:348)
==3257==    by 0x48D1A7F: gst_element_factory_make (gstelementfactory.c:445)
==3257==
==3257== LEAK SUMMARY:
==3257==    definitely lost: 184 bytes in 1 blocks
==3257==    indirectly lost: 32 bytes in 2 blocks
==3257==      possibly lost: 0 bytes in 0 blocks
==3257==    still reachable: 119,351 bytes in 2,018 blocks
==3257==                       of which reachable via heuristic:
==3257==                         length64           : 7,784 bytes in 188 blocks
==3257==                         newarray           : 1,648 bytes in 23 blocks

This is happening because the shm allocator is not released. The cause seems to be because there is no unref after acquiring the allocator with gst_wl_shm_allocator_get (). This function is called in gstwaylandsink.c and wlwindow.c , and calls gst_allocator_find () internally, and the specification states that unref is necessary after use. https://developer.gnome.org/gstreamer/stable/GstAllocator.html#gst-allocator-find
Applying the attached patch can solve this leak.

I double checked in gstreamer 1.6.3 and there there is no unref for the shm allocator there either, however executing valgrind in 1.6.3 will not detect leak of the allocator. Looking at the log of refcount, we can confirm that refcount itself remains. Did something change inside the allocator or elsewhere?
Comment 1 Nicolas Dufresne (ndufresne) 2017-12-06 20:37:29 UTC
Review of attachment 363186 [details] [review]:

Looks good to me.
Comment 2 Nicolas Dufresne (ndufresne) 2017-12-06 20:47:57 UTC
Thanks.

commit 816d115317c522c87297109de781c63b16146493 (HEAD -> master)
Author: Shinya Saito <ssaito@igel.co.jp>
Date:   Wed Nov 1 18:05:26 2017 +0900

    waylandsink: Fix memory leak of shm allocator.
    
    gst_allocator_find() needs gst_object_unref() after usage.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=790042

commit 33801fe0951c0829f2ed7a35c3451bb7a9d86027 (HEAD -> 1.12, origin/1.12)
Author: Shinya Saito <ssaito@igel.co.jp>
Date:   Wed Nov 1 18:05:26 2017 +0900

    waylandsink: Fix memory leak of shm allocator.
    
    gst_allocator_find() needs gst_object_unref() after usage.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=790042
Comment 3 Shinya Saito 2017-12-07 01:45:01 UTC
Thank you for accepting patches.

However, it seems that the correction has not been reflected correctly.


commit 816d115317c522c87297109de781c63b16146493

@@ -579,8 +582,10 @@ gst_wayland_sink_propose_allocation (GstBaseSink * bsink, G
stQuery * query)
   if (pool)
     g_object_unref (pool);
 
+  alloc = gst_wl_shm_allocator_get ();
   gst_query_add_allocation_param (query, gst_wl_shm_allocator_get (), NULL);
   gst_query_add_allocation_meta (query, GST_VIDEO_META_API_TYPE, NULL);
+  g_object_unref (alloc);


Since the previous gst_wl_shm_allocator_get() remains, this patch can not fix the leak. This part should be replaced with the alloc.
Comment 4 Nicolas Dufresne (ndufresne) 2017-12-07 01:46:59 UTC
Merge error, thanks for spotting.
Comment 5 Nicolas Dufresne (ndufresne) 2017-12-07 01:51:39 UTC
commit c04aba241e2595bcafcd587b70bcb3724bbd3c35 (HEAD -> master, origin/master, origin/HEAD)
Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Date:   Wed Dec 6 20:50:31 2017 -0500

    waylandsink: Fix memory leak of shm allocator
    
    This fixes conflict resolution error introduced in commit:
    
      816d115317c522c87297109de781c63b16146493
    
    http://bugzilla.gnome.org/show_bug.cgi?id=790042