GNOME Bugzilla – Bug 561825
Problem with RTCP thread using freed objects
Last modified: 2009-08-31 14:52:01 UTC
The following is a (lightly edited) transcript of an IRC discussion I had with Wim where I tried to described a problem we are seeing with the RTCP thread in the gstrtpbin: Saur: We are about to setup a new pipeline which contains an gstrtpbin. Saur: What happens is that a new session is created with create_session(), with the session element set to PLAYING (the rest of the pipeline at this time is in the NULL state...) Saur: Then we set the pipeline to READY, which leads to that the session element above makes the PLAYING->READY transition. wtay: yeah Saur: This in turn leads to that we call stop_rtcp_thread() wtay: that can be improved Saur: and then proceed to call the default state transition function for elements. Saur: Now there are things happening concurrently in the RTCP thread and in the main thread. Saur: In the RTCP thread we happen to be in gst_rtp_session_send_rtcp() and have just gotten a caps to set on our RTCP buffer. Saur: Meanwhile in the main thread we are doing the PAUSED->READY transition which involves setting all capses to NULL on the pads... Saur: And boom, gst_buffer_set_caps() fails because the caps has been deleted... wtay: yes Saur: One way I see to solve this is to make stop_rtcp_thread() actually wait until thread_stopped has been set Saur: But I am not sure whether this may lock up in case the gst_pad_push() in gst_rtp_session_send_rtcp() hangs... wtay: or just not set the state to PAUSED/PLAYING when adding the session Saur: Yeah, but you would still have the same situation, but when taking down the pipeline... wtay: yes but then we can sync the thread, like we do everywhere else wtay: because any push would already have been unblocked wtay: and we avoid the useless start/stop rtcp thread Saur: Why do you set the elements to PLAYING anyway? wtay: because it's a quick hack to make it work Saur: :) Saur: Hmm, how about I lock the session in gst_rtp_session_send_rtcp() (except when calling gst_pad_push()). Think that would solve the problem? wtay: not really because it's already unlocked when the push happens Saur: Yeah, but then the caps is already set on the buffer, which should now hold a reference to it. Saur: Or there may be problems with gst_pad_push() on an element which state is simultaneously being changed?
commit a74c385b7b7d12d0a312de6e59c845ed25836c6e Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Mon Aug 31 16:34:14 2009 +0200 rtpsession: use proper locking for pads and caps Use the sesion lock and shotdown variable to protect and ref the pads we are going to push on. fixes #561825