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 688707 - rtsp-media: bus watch can outlive the rtsp-media object
rtsp-media: bus watch can outlive the rtsp-media object
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other Linux
: Normal normal
: 1.2.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-11-20 08:27 UTC by David Svensson Fors
Modified: 2014-02-25 22:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Valgrind run (12.73 KB, text/plain)
2012-11-20 08:27 UTC, David Svensson Fors
  Details
Remove bus watch before finalizing (2.51 KB, patch)
2012-11-20 08:28 UTC, David Svensson Fors
none Details | Review

Description David Svensson Fors 2012-11-20 08:27:40 UTC
Created attachment 229447 [details]
Valgrind run

In gst-rtsp-server, I run a unit test in Valgrind:

GST_CHECKS=test_describe make -C tests/check gst/rtspserver.valgrind

I get several invalid reads. Please see the attached file. These invalid reads come in the bus watch (bus_message) in rtsp-media.c. The RTSPClient has shut down. Its finalize method has unreffed the last reference to its RTSPMedia, which also gets finalized. gst_rtsp_media_finalize clears the state_lock GRecMutex. Then, a (last) message comes to the bus_message handler. There, state_lock is accessed after it has been cleared.
Comment 1 David Svensson Fors 2012-11-20 08:28:50 UTC
Created attachment 229448 [details] [review]
Remove bus watch before finalizing

A suggested fix is attached:

* A GDestroyNotify function is set for the bus watch in gst_rtsp_media_prepare.
* An extra media ref is added for the bus watch. This extra ref is unreffed by the GDestroyNotify function.
* gst_rtsp_media_unprepare destroys the source so the bus watch is removed.
* GstRTSPClient, which calls gst_rtsp_media_prepare, also calls gst_rtsp_media_unprepare before unreffing the media.

This way, the bus watch will be removed before the media is finalized.
Comment 2 Wim Taymans 2012-11-20 08:47:32 UTC
I changed it so that the source is removed in finish_unprepare in the case of clean shutdown.

commit 989f004e24e6e6e8d8a0a041849d9ce593403666
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Tue Nov 20 09:42:51 2012 +0100

    media: unref source in finish_unprepare
    
    The source is created in prepare, unref it in finish_unprepare.
    
    See https://bugzilla.gnome.org/show_bug.cgi?id=688707

commit 01973c924d3fc850b812a7aaf1b2691a8fd01ae0
Author: David Svensson Fors <davidsf@axis.com>
Date:   Mon Nov 19 15:47:08 2012 +0100

    rtsp-media: remove bus watch before finalizing
    
    * A GDestroyNotify function is set for the bus watch in gst_rtsp_media_prepare.
    * An extra media ref is added for the bus watch. This extra ref is unreffed by
    the GDestroyNotify function.
    * gst_rtsp_media_unprepare destroys the source so the bus watch is removed.
    * GstRTSPClient, which calls gst_rtsp_media_prepare, also calls
    gst_rtsp_media_unprepare before unreffing the media.
    
    This way, the bus watch will be removed before the media is finalized.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=688707