rtpsession: Fix on-feedback-rtcp race
If there is an external source which is about to timeout and be removed
from the source hashtable and we receive feedback RTCP packet with the
media ssrc of the source, we unlock the session in
rtp_session_process_feedback before emitting 'on-feedback-rtcp' signal
allowing rtcp timer to kick in and grab the lock. It will get rid of
the source and rtp_session_process_feedback will be left with RTPSource
with ref count 0.
The fix is to grab the ref to the RTPSource object in
rtp_session_process_feedback.
https://bugzilla.gnome.org/show_bug.cgi?id=795139
diff --git a/gst/rtpmanager/rtpsession.c b/gst/rtpmanager/rtpsession.c
index ff0c09a..7d76703 100644
--- a/gst/rtpmanager/rtpsession.c
+++ b/gst/rtpmanager/rtpsession.c
@@ -2731,6 +2731,9 @@
if (sess->scheduled_bye && src && RTP_SOURCE_IS_MARKED_BYE (src))
return;
+ if (src)
+ g_object_ref (src);
+
fci_data = gst_rtcp_packet_fb_get_fci (packet);
fci_length = gst_rtcp_packet_fb_get_fci_length (packet) * sizeof (guint32);
@@ -2798,6 +2801,9 @@
break;
}
}
+
+ if (src)
+ g_object_unref (src);
}
/**
diff --git a/tests/check/elements/rtpsession.c b/tests/check/elements/rtpsession.c
index 94925ee..9cf1ae8 100644
--- a/tests/check/elements/rtpsession.c
+++ b/tests/check/elements/rtpsession.c
@@ -705,6 +705,88 @@
GST_END_TEST;
+typedef struct
+{
+ GCond *cond;
+ GMutex *mutex;
+ gboolean fired;
+} FeedbackRTCPCallbackData;
+
+static void
+feedback_rtcp_cb (GstElement * element, guint fbtype, guint fmt,
+ guint sender_ssrc, guint media_ssrc, GstBuffer * fci,
+ FeedbackRTCPCallbackData * cb_data)
+{
+ g_mutex_lock (cb_data->mutex);
+ cb_data->fired = TRUE;
+ g_cond_wait (cb_data->cond, cb_data->mutex);
+ g_mutex_unlock (cb_data->mutex);
+}
+
+static void *
+send_feedback_rtcp (SessionHarness * h)
+{
+ GstRTCPPacket packet;
+ GstRTCPBuffer rtcp = GST_RTCP_BUFFER_INIT;
+ GstBuffer *buffer = gst_rtcp_buffer_new (1000);
+
+ fail_unless (gst_rtcp_buffer_map (buffer, GST_MAP_READWRITE, &rtcp));
+ fail_unless (gst_rtcp_buffer_add_packet (&rtcp, GST_RTCP_TYPE_PSFB, &packet));
+ gst_rtcp_packet_fb_set_type (&packet, GST_RTCP_PSFB_TYPE_PLI);
+ gst_rtcp_packet_fb_set_fci_length (&packet, 0);
+ gst_rtcp_packet_fb_set_media_ssrc (&packet, 0xABE2B0B);
+ gst_rtcp_packet_fb_set_media_ssrc (&packet, 0xDEADBEEF);
+ gst_rtcp_buffer_unmap (&rtcp);
+ fail_unless_equals_int (GST_FLOW_OK, session_harness_recv_rtcp (h, buffer));
+
+ return NULL;
+}
+
+GST_START_TEST (test_feedback_rtcp_race)
+{
+ SessionHarness *h = session_harness_new ();
+
+ GCond cond;
+ GMutex mutex;
+ FeedbackRTCPCallbackData cb_data;
+ GThread *send_rtcp_thread;
+
+ g_cond_init (&cond);
+ g_mutex_init (&mutex);
+ cb_data.cond = &cond;
+ cb_data.mutex = &mutex;
+ cb_data.fired = FALSE;
+ g_signal_connect (h->internal_session, "on-feedback-rtcp",
+ G_CALLBACK (feedback_rtcp_cb), &cb_data);
+
+ /* Push RTP buffer making external source with SSRC=0xDEADBEEF */
+ fail_unless_equals_int (GST_FLOW_OK, session_harness_recv_rtp (h,
+ generate_test_buffer (0, 0xDEADBEEF)));
+
+ /* Push feedback RTCP with media SSRC=0xDEADBEEF */
+ send_rtcp_thread = g_thread_new (NULL, (GThreadFunc) send_feedback_rtcp, h);
+
+ /* Waiting for feedback RTCP callback to fire */
+ while (!cb_data.fired)
+ g_usleep (G_USEC_PER_SEC / 100);
+
+ /* While send_rtcp_thread thread is waiting for our signal
+ advance the clock by 30sec triggering removal of 0xDEADBEEF,
+ as if the source was inactive for too long */
+ session_harness_advance_and_crank (h, GST_SECOND * 30);
+ gst_buffer_unref (session_harness_pull_rtcp (h));
+
+ /* Let send_rtcp_thread finish */
+ g_mutex_lock (&mutex);
+ g_cond_signal (&cond);
+ g_mutex_unlock (&mutex);
+ g_thread_join (send_rtcp_thread);
+
+ session_harness_free (h);
+}
+
+GST_END_TEST;
+
static Suite *
rtpsession_suite (void)
{
@@ -718,8 +800,8 @@
tcase_add_test (tc_chain, test_receive_rtcp_app_packet);
tcase_add_test (tc_chain, test_dont_lock_on_stats);
tcase_add_test (tc_chain, test_ignore_suspicious_bye);
-
tcase_add_test (tc_chain, test_illegal_rtcp_fb_packet);
+ tcase_add_test (tc_chain, test_feedback_rtcp_race);
return s;
}
