rtsp-stream: Removing invalid transports returns false

When removing transports an assertion was that the transports passed in
for removal are present in the list, however that can't be assumed.
As an example if a transport was removed from a thread running
send_tcp_message, the main thread can try to remove the same transport
again if it gets a handle_pause_request. This will not effect the
transport list but it will effect n_tcp_transports as it will be
decrement and then have the wrong value.
This commit is contained in:
Adam x Nilsson 2019-11-01 12:01:41 +01:00 committed by Adam Nilsson
parent c2d182de05
commit 9c5ca231a6
3 changed files with 82 additions and 12 deletions

View file

@ -28,7 +28,7 @@
* to handle the RTP and RTCP packets from the stream, for example when they
* need to be sent over TCP.
*
* With gst_rtsp_stream_transport_set_active() the transports are added and
* With gst_rtsp_stream_transport_set_active() the transports are added and
* removed from the stream.
*
* A #GstRTSPStream will call gst_rtsp_stream_transport_keep_alive() when RTCP
@ -71,7 +71,6 @@ struct _GstRTSPStreamTransportPrivate
GstRTSPKeepAliveFunc keep_alive;
gpointer ka_user_data;
GDestroyNotify ka_notify;
gboolean active;
gboolean timed_out;
GstRTSPMessageSentFunc message_sent;
@ -526,17 +525,11 @@ gst_rtsp_stream_transport_set_active (GstRTSPStreamTransport * trans,
priv = trans->priv;
if (priv->active == active)
return FALSE;
if (active)
res = gst_rtsp_stream_add_transport (priv->stream, trans);
else
res = gst_rtsp_stream_remove_transport (priv->stream, trans);
if (res)
priv->active = active;
return res;
}

View file

@ -4451,10 +4451,18 @@ update_transport (GstRTSPStream * stream, GstRTSPStreamTransport * trans,
const GstRTSPTransport *tr;
gchar *dest;
gint min, max;
GList *tr_element;
tr = gst_rtsp_stream_transport_get_transport (trans);
dest = tr->destination;
tr_element = g_list_find (priv->transports, trans);
if (add && tr_element)
return TRUE;
else if (!add && !tr_element)
return FALSE;
switch (tr->lower_transport) {
case GST_RTSP_LOWER_TRANS_UDP_MCAST:
{
@ -4483,9 +4491,9 @@ update_transport (GstRTSPStream * stream, GstRTSPStreamTransport * trans,
if (!remove_mcast_client_addr (stream, dest, min, max))
GST_WARNING_OBJECT (stream,
"Failed to remove multicast address: %s:%d-%d", dest, min, max);
priv->transports = g_list_delete_link (priv->transports, tr_element);
remove_client (priv->mcast_udpsink[0], priv->mcast_udpsink[1], dest,
min, max);
priv->transports = g_list_remove (priv->transports, trans);
}
break;
}
@ -4507,8 +4515,8 @@ update_transport (GstRTSPStream * stream, GstRTSPStreamTransport * trans,
priv->transports = g_list_prepend (priv->transports, trans);
} else {
GST_INFO ("removing %s:%d-%d", dest, min, max);
priv->transports = g_list_delete_link (priv->transports, tr_element);
remove_client (priv->udpsink[0], priv->udpsink[1], dest, min, max);
priv->transports = g_list_remove (priv->transports, trans);
}
priv->transports_cookie++;
break;
@ -4520,7 +4528,7 @@ update_transport (GstRTSPStream * stream, GstRTSPStreamTransport * trans,
priv->n_tcp_transports++;
} else {
GST_INFO ("removing TCP %s", tr->destination);
priv->transports = g_list_remove (priv->transports, trans);
priv->transports = g_list_delete_link (priv->transports, tr_element);
priv->n_tcp_transports--;
}
priv->transports_cookie++;
@ -4601,7 +4609,8 @@ on_message_sent (GstRTSPStreamTransport * trans, gpointer user_data)
* @trans: (transfer none): a #GstRTSPStreamTransport
*
* Add the transport in @trans to @stream. The media of @stream will
* then also be send to the values configured in @trans.
* then also be send to the values configured in @trans. Adding the
* same transport twice will not add it a second time.
*
* @stream must be joined to a bin.
*

View file

@ -575,6 +575,72 @@ GST_START_TEST (test_multicast_client_address_invalid)
GST_END_TEST;
static void
add_transports (gboolean add_twice)
{
GstRTSPTransport *transport;
GstRTSPStream *stream;
GstRTSPStreamTransport *tr;
GstPad *srcpad;
GstElement *pay;
GstBin *bin;
GstElement *rtpbin;
fail_unless (gst_rtsp_transport_new (&transport) == GST_RTSP_OK);
transport->lower_transport = GST_RTSP_LOWER_TRANS_TCP;
transport->destination = g_strdup ("127.0.0.1");
srcpad = gst_pad_new ("testsrcpad", GST_PAD_SRC);
fail_unless (srcpad != NULL);
pay = gst_element_factory_make ("rtpgstpay", "testpayloader");
fail_unless (pay != NULL);
stream = gst_rtsp_stream_new (0, pay, srcpad);
fail_unless (stream != NULL);
gst_object_unref (pay);
gst_object_unref (srcpad);
rtpbin = gst_element_factory_make ("rtpbin", "testrtpbin");
fail_unless (rtpbin != NULL);
bin = GST_BIN (gst_bin_new ("testbin"));
fail_unless (bin != NULL);
fail_unless (gst_bin_add (bin, rtpbin));
/* TCP transport */
gst_rtsp_stream_set_protocols (stream, GST_RTSP_LOWER_TRANS_TCP);
fail_unless (gst_rtsp_stream_join_bin (stream, bin, rtpbin, GST_STATE_NULL));
tr = gst_rtsp_stream_transport_new (stream, transport);
fail_unless (tr);
if (add_twice) {
fail_unless (gst_rtsp_stream_add_transport (stream, tr));
fail_unless (gst_rtsp_stream_add_transport (stream, tr));
fail_unless (gst_rtsp_stream_remove_transport (stream, tr));
} else {
fail_unless (gst_rtsp_stream_add_transport (stream, tr));
fail_unless (gst_rtsp_stream_remove_transport (stream, tr));
fail_if (gst_rtsp_stream_remove_transport (stream, tr));
}
fail_unless (gst_rtsp_transport_free (transport) == GST_RTSP_OK);
fail_unless (gst_rtsp_stream_leave_bin (stream, bin, rtpbin));
gst_object_unref (bin);
gst_object_unref (stream);
}
GST_START_TEST (test_add_transport_twice)
{
add_transports (TRUE);
}
GST_END_TEST;
GST_START_TEST (test_remove_transport_twice)
{
add_transports (FALSE);
}
GST_END_TEST;
static Suite *
rtspstream_suite (void)
{
@ -592,6 +658,8 @@ rtspstream_suite (void)
tcase_add_test (tc, test_tcp_transport);
tcase_add_test (tc, test_multicast_client_address);
tcase_add_test (tc, test_multicast_client_address_invalid);
tcase_add_test (tc, test_add_transport_twice);
tcase_add_test (tc, test_remove_transport_twice);
return s;
}