rtsp-client: Remove backlog limit while processings requests

If the backlog limit is kept two cases of deadlocks may be
encountered when streaming over TCP. Without the backlog
limit this deadlocks can not happen, at the expence of
memory usage.

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=737631

1 files changed, 38 insertions, 9 deletions

diff --git a/gst/rtsp-server/rtsp-client.c b/gst/rtsp-server/rtsp-client.c
index df02258..aa0ab98 100644
--- a/gst/rtsp-server/rtsp-client.c
+++ b/gst/rtsp-server/rtsp-client.c
@@ -964,7 +964,6 @@ bad_request:
 static gboolean
 handle_pause_request (GstRTSPClient * client, GstRTSPContext * ctx)
 {
-  GstRTSPClientPrivate *priv = client->priv;
   GstRTSPSession *session;
   GstRTSPClientClass *klass;
   GstRTSPSessionMedia *sessmedia;
@@ -1000,11 +999,6 @@ handle_pause_request (GstRTSPClient * client, GstRTSPContext * ctx)
       rtspstate != GST_RTSP_STATE_RECORDING)
     goto invalid_state;
 
-  /* No limit on watch queue because else we might be blocking in the appsink
-   * render method and the PAUSE below will hang */
-  if (priv->watch != NULL)
-    gst_rtsp_watch_set_send_backlog (priv->watch, 0, 0);
-
   /* then pause sending */
   gst_rtsp_session_media_set_state (sessmedia, GST_STATE_PAUSED);
 
@@ -1015,9 +1009,6 @@ handle_pause_request (GstRTSPClient * client, GstRTSPContext * ctx)
 
   send_message (client, ctx, ctx->response, FALSE);
 
-  if (priv->watch != NULL)
-    gst_rtsp_watch_set_send_backlog (priv->watch, 0, WATCH_BACKLOG_SIZE);
-
   /* the state is now READY */
   gst_rtsp_session_media_set_rtsp_state (sessmedia, GST_RTSP_STATE_READY);
 
@@ -2375,6 +2366,37 @@ handle_request (GstRTSPClient * client, GstRTSPMessage * request)
   if (!check_request_requirements (ctx->request, &unsupported_reqs))
     goto unsupported_requirement;
 
+  /* the backlog must be unlimited while processing requests.
+   * the causes of this are two cases of deadlocks while streaming over TCP:
+   *
+   * 1. consider the scenario where the media pipeline's streaming thread
+   * is blocking in the appsink (taking the appsink's preroll lock) because
+   * the backlog is full. when a PAUSE request is received by the RTSP
+   * client thread then the the state of the session media ought to change
+   * to PAUSED. while most elements in the pipeline can change state this
+   * can never happen for the appsink since its preroll lock is taken by
+   * another thread.
+   *
+   * 2. consider the scenario where the media pipeline's streaming thread
+   * is blocking in the appsink new_sample callback (taking the send lock
+   * in RTSP client) because the backlog is full. when e.g. a GET request
+   * is received by the RTSP client thread then a response ought to be sent
+   * but this can never happen since it requires taking the send lock
+   * already taken by another thread.
+   *
+   * the reason that the backlog is never emptied is that the source used
+   * for dequeing messages from the backlog is never dispatched because it
+   * is attached to the same mainloop as the source receving RTSP requests and
+   * therefore run by the RTSP client thread which is alreayd blocking.
+   *
+   * without significant changes the easiest way to cope with this is to
+   * not block indefinitely when the backlog is full, but rather let the
+   * backlog grow in size. this in effect means that there can not be any
+   * upper boundary on its size.
+   */
+  if (priv->watch != NULL)
+    gst_rtsp_watch_set_send_backlog (priv->watch, 0, 0);
+
   /* now see what is asked and dispatch to a dedicated handler */
   switch (method) {
     case GST_RTSP_OPTIONS:
@@ -2404,12 +2426,19 @@ handle_request (GstRTSPClient * client, GstRTSPMessage * request)
     case GST_RTSP_ANNOUNCE:
     case GST_RTSP_RECORD:
     case GST_RTSP_REDIRECT:
+      if (priv->watch != NULL)
+        gst_rtsp_watch_set_send_backlog (priv->watch, 0, WATCH_BACKLOG_SIZE);
       goto not_implemented;
     case GST_RTSP_INVALID:
     default:
+      if (priv->watch != NULL)
+        gst_rtsp_watch_set_send_backlog (priv->watch, 0, WATCH_BACKLOG_SIZE);
       goto bad_request;
   }
 
+  if (priv->watch != NULL)
+    gst_rtsp_watch_set_send_backlog (priv->watch, 0, WATCH_BACKLOG_SIZE);
+
 done:
   if (ctx == &sctx)
     gst_rtsp_context_pop_current (ctx);