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 709730 - GstRTSPThreadPool leaks threads
GstRTSPThreadPool leaks threads
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other Linux
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-10-09 13:33 UTC by Ognyan Tonchev (redstar_)
Modified: 2014-06-22 13:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
One ref is always kept for the thread's main loop and every call to gst_rtsp_thread_pool_get_thread () results in a new ref which is dropped from gst_thread_stop (). (2.33 KB, patch)
2013-10-09 13:33 UTC, Ognyan Tonchev (redstar_)
needs-work Details | Review
Unit test for GstRTSPThreadPool (3.38 KB, patch)
2013-10-09 13:45 UTC, Ognyan Tonchev (redstar_)
none Details | Review
fixing small bug in the unit test patch, free pool using g_object_unreaf instead of gst_object_unref (3.38 KB, patch)
2013-10-11 07:37 UTC, Ognyan Tonchev (redstar_)
committed Details | Review
Fixes annotation in my previous patch - (transfer full) for the thread parameter (2.38 KB, patch)
2013-10-15 10:15 UTC, Ognyan Tonchev (redstar_)
committed Details | Review

Description Ognyan Tonchev (redstar_) 2013-10-09 13:33:44 UTC
Created attachment 256802 [details] [review]
One ref is always kept for the thread's main loop and every call to gst_rtsp_thread_pool_get_thread () results in a new ref which is dropped from gst_thread_stop ().

All reused client threads are currently leaked.

I am also proposing a patch which solves the leak. One ref is always kept for the thread's main loop and every call to gst_rtsp_thread_pool_get_thread () results in a new ref which is dropped from gst_thread_stop ().
Comment 1 Ognyan Tonchev (redstar_) 2013-10-09 13:45:56 UTC
Created attachment 256806 [details] [review]
Unit test for GstRTSPThreadPool

Here is a unit test which reproduces the leak described in the ticket, but it also reveals another bug.
If gst_rtsp_thread_stop() is called before the main loop for the thread is started it will do nothing and the main loop will eventually run. This can be solved with a mutex, flag and idle source but may be there is a nicer solution to that problem?
Comment 2 Ognyan Tonchev (redstar_) 2013-10-11 07:37:48 UTC
Created attachment 256972 [details] [review]
fixing small bug in the unit test patch, free pool using g_object_unreaf instead of gst_object_unref
Comment 3 Sebastian Dröge (slomo) 2013-10-14 20:01:59 UTC
Review of attachment 256802 [details] [review]:

::: gst/rtsp-server/rtsp-thread-pool.c
@@ +147,3 @@
  *
+ * Stop and unref @thread. When no threads are using the mainloop, the thread
+ * will be stopped and the final ref to @thread will be released.

This should get an corresponding annotation then if I understand it correctly... (transfer full) for the thread parameter.
Comment 4 Ognyan Tonchev (redstar_) 2013-10-15 10:15:36 UTC
Created attachment 257338 [details] [review]
Fixes annotation in my previous patch - (transfer full) for the thread parameter
Comment 5 Sebastian Dröge (slomo) 2013-10-30 18:02:55 UTC
commit 78e5a9148e6ba8d80fd3f2e78043abd7a434ec23
Author: Ognyan Tonchev <ognyan@axis.com>
Date:   Wed Oct 9 15:25:10 2013 +0200

    thread-pool: Fix thread leak when reusing threads
    
    https://bugzilla.gnome.org/show_bug.cgi?id=709730


commit fa7898b0a61bed689cff7f57285be561d7ab5028
Author: Ognyan Tonchev <ognyan@axis.com>
Date:   Wed Oct 9 15:19:12 2013 +0200

    thread-pool: Add unit test for the thread pools
    
    https://bugzilla.gnome.org/show_bug.cgi?id=710228