rtsp-client: cleanup transports during TEARDOWN

When tunneling RTP over RTSP the stream transports are stored in a hash
table in the GstRTSPClientPrivate struct. They are used for, among other
things, mapping channel id to stream transports when receiving data from
the client. The stream tranports are created and added to the hash table
in handle_setup_request(), but unfortuately they are not removed in
handle_teardown_request(). This means that if the client sends data on
the RTSP connection after it has sent the TEARDOWN, which is often the
case when audio backchannel is enabled, handle_data() will still be able
to map the channel to a session transport and pass the data along to it.
Which eventually leads to a failing assert in gst_rtsp_stream_recv_rtp()
because the stream is no longer joined to a bin.
We avoid this by removing the stream transports from the hash table when
we handle the TEARDOWN request.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-rtsp-server/-/merge_requests/184>

1 files changed, 60 insertions, 0 deletions

diff --git a/gst/rtsp-server/rtsp-client.c b/gst/rtsp-server/rtsp-client.c
index a83725c..8312e3c 100644
--- a/gst/rtsp-server/rtsp-client.c
+++ b/gst/rtsp-server/rtsp-client.c
@@ -1371,6 +1371,55 @@ pre_signal_accumulator (GSignalInvocationHint * ihint, GValue * return_accu,
   return TRUE;
 }
 
+/* The cleanup_transports function is called from handle_teardown_request() to
+ * remove any stream transports from the newly closed session that were added to
+ * priv->transports in handle_setup_request(). This is done to avoid forwarding
+ * data from the client on a channel that we just closed.
+ */
+static void
+cleanup_transports (GstRTSPClient * client, GPtrArray * transports)
+{
+  GstRTSPClientPrivate *priv = client->priv;
+  GstRTSPStreamTransport *stream_transport;
+  const GstRTSPTransport *rtsp_transport;
+  guint i;
+
+  GST_LOG_OBJECT (client, "potentially removing %u transports",
+      transports->len);
+
+  for (i = 0; i < transports->len; i++) {
+    stream_transport = g_ptr_array_index (transports, i);
+    if (stream_transport == NULL) {
+      GST_LOG_OBJECT (client, "stream transport %u is NULL, continue", i);
+      continue;
+    }
+
+    rtsp_transport = gst_rtsp_stream_transport_get_transport (stream_transport);
+    if (rtsp_transport == NULL) {
+      GST_LOG_OBJECT (client, "RTSP transport %u is NULL, continue", i);
+      continue;
+    }
+
+    /* priv->transport only stores transports where RTP is tunneled over RTSP */
+    if (rtsp_transport->lower_transport == GST_RTSP_LOWER_TRANS_TCP) {
+      if (!g_hash_table_remove (priv->transports,
+              GINT_TO_POINTER (rtsp_transport->interleaved.min))) {
+        GST_WARNING_OBJECT (client,
+            "failed removing transport with key '%d' from priv->transports",
+            rtsp_transport->interleaved.min);
+      }
+      if (!g_hash_table_remove (priv->transports,
+              GINT_TO_POINTER (rtsp_transport->interleaved.max))) {
+        GST_WARNING_OBJECT (client,
+            "failed removing transport with key '%d' from priv->transports",
+            rtsp_transport->interleaved.max);
+      }
+    } else {
+      GST_LOG_OBJECT (client, "transport %u not RTP/RTSP, skip it", i);
+    }
+  }
+}
+
 static gboolean
 handle_teardown_request (GstRTSPClient * client, GstRTSPContext * ctx)
 {
@@ -1384,6 +1433,7 @@ handle_teardown_request (GstRTSPClient * client, GstRTSPContext * ctx)
   gint matched;
   gboolean keep_session;
   GstRTSPStatusCode sig_result;
+  GPtrArray *session_media_transports;
 
   if (!ctx->session)
     goto no_session;
@@ -1419,6 +1469,10 @@ handle_teardown_request (GstRTSPClient * client, GstRTSPContext * ctx)
     goto sig_failed;
   }
 
+  /* get a reference to the transports in the session media so we can clean up
+   * our priv->transports before returning */
+  session_media_transports = gst_rtsp_session_media_get_transports (sessmedia);
+
   /* we emit the signal before closing the connection */
   g_signal_emit (client, gst_rtsp_client_signals[SIGNAL_TEARDOWN_REQUEST],
       0, ctx);
@@ -1444,6 +1498,12 @@ handle_teardown_request (GstRTSPClient * client, GstRTSPContext * ctx)
   gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
+  /* remove all transports that were present in the session media which we just
+   * unmanaged from the priv->transports array, so we do not try to handle data
+   * on channels that were just closed */
+  cleanup_transports (client, session_media_transports);
+  g_ptr_array_unref (session_media_transports);
+
   return TRUE;
 
   /* ERRORS */