GNOME Bugzilla – Bug 742954
Crash when two treads are in handle_new_sample at the same time.
Last modified: 2015-01-16 11:53:22 UTC
Created attachment 294571 [details] [review] patch in gst-rtsp-server There is a possibility to crash if two threads are in function handle_new_sample in file rtsp-stream.c at the same time. One thread for rtp and one thread for rtcp. One thread is manipulating the list tr_cache and even free it. The other one is iterating the list. We have only seen this crash once. But if insert 1s sleeps like this. static void clear_tr_cache (GstRTSPStreamPrivate * priv) { g_list_foreach (priv->tr_cache, (GFunc) g_object_unref, NULL); g_list_free (priv->tr_cache); g_usleep(1000000); priv->tr_cache = NULL; } static GstFlowReturn handle_new_sample (GstAppSink * sink, gpointer user_data) , , , g_mutex_unlock (&priv->lock); g_usleep(1000000); for (walk = priv->tr_cache; walk; walk = g_list_next (walk)) { GstRTSPStreamTransport *tr = (GstRTSPStreamTransport *) walk->data; if (is_rtp) { gst_rtsp_stream_transport_send_rtp (tr, buffer); } else { gst_rtsp_stream_transport_send_rtcp (tr, buffer); } } And repeatedly open and close a rtsp tunneled sessions it's easy to reproduce.
The adding of sleeps is wrong above, the second sleep is only when rtcp. like this static GstFlowReturn handle_new_sample (GstAppSink * sink, gpointer user_data) , , , g_mutex_unlock (&priv->lock); if(!is_rtp) g_usleep(1000000); for (walk = priv->tr_cache; walk; walk = g_list_next (walk)) { GstRTSPStreamTransport *tr = (GstRTSPStreamTransport *) walk->data; if (is_rtp) { gst_rtsp_stream_transport_send_rtp (tr, buffer); } else { gst_rtsp_stream_transport_send_rtcp (tr, buffer); } }
This seems dangerous if transport_send_*() blocks. Why is there this tr_cache at all? It just seems to be a copy of the transports list. The only explanation I would have is that it's there to not have a mutex locked while sending the actual packets.
Created attachment 294659 [details] [review] new patch gst-rtsp-server New patch with a new solution. Now use two copy's instead of just one. Now rtp have one copy and rtcp have one.
commit 0d2de69db99670451bc7a39d921dc5636fd94cec Author: Göran Jönsson <goranjn@axis.com> Date: Fri Jan 16 11:10:20 2015 +0100 rtsp-stream: Have one copy of the transports cache for RTP and RTCP each Fixes crash when two threads access handle_new_sample() at the same time, one for RTP, one for RTCP. Otherwise, when iterating over the transports cache, it might be modified by another thread at the same time if the transports cookie has changed. https://bugzilla.gnome.org/show_bug.cgi?id=742954