rtsp-server: media-factory: Make sure a shared media is actually still usable

Previously it was possible that a shared media was just in the process
of being unprepared because the last client disappeared, while another
client retrieved it from the cache and then tried to use it. Unless the
media was reusable this would've then failed unnecessarily.

To avoid this it is necessary to lock the media directly in
gst_rtsp_media_factory_construct() and return a locked media. After
locking the cached media it is necessary to check if the media was ever
unprepared or is actually reusable and based on that either reuse it or
create a new media.

This minimally changes the gst_rtsp_media_factory_construct() API to
always return a locked media, and adds a new
gst_rtsp_media_can_be_shared() function to check if a media can actually
be shared in practice.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4606>

girs/GstRtspServer-1.0.gir

@@ -3562,6 +3562,25 @@ when the media was not in the suspended state.</doc>
           </instance-parameter>
         </parameters>
       </virtual-method>
+      <method name="can_be_shared" c:identifier="gst_rtsp_media_can_be_shared" version="1.24">
+        <doc xml:space="preserve" filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.c">Check if the pipeline for @media can be shared between multiple clients.
+
+This checks if the media is shareable and whether it is either reusable or
+was never unprepared before.
+
+The function must be called with gst_rtsp_media_lock().</doc>
+        <source-position filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.h"/>
+        <return-value transfer-ownership="none">
+          <doc xml:space="preserve" filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.c">%TRUE if the media can be shared between clients.</doc>
+          <type name="gboolean" c:type="gboolean"/>
+        </return-value>
+        <parameters>
+          <instance-parameter name="media" transfer-ownership="none">
+            <doc xml:space="preserve" filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.c">a #GstRTSPMedia</doc>
+            <type name="RTSPMedia" c:type="GstRTSPMedia*"/>
+          </instance-parameter>
+        </parameters>
+      </method>
       <method name="collect_streams" c:identifier="gst_rtsp_media_collect_streams">
         <doc xml:space="preserve" filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.c">Find all payloader elements, they should be named pay\%d in the
 element of @media, and create #GstRTSPStreams for them.
@@ -4093,7 +4112,12 @@ unpreparing.</doc>
         </parameters>
       </method>
       <method name="is_shared" c:identifier="gst_rtsp_media_is_shared">
-        <doc xml:space="preserve" filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.c">Check if the pipeline for @media can be shared between multiple clients.</doc>
+        <doc xml:space="preserve" filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.c">Check if the pipeline for @media can be shared between multiple clients in
+theory. This simply returns the value set via gst_rtsp_media_set_shared().
+
+To know if a media can be shared in practice, i.e. if it's shareable and
+either reusable or was never unprepared before, use
+gst_rtsp_media_can_be_shared().</doc>
         <source-position filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.h"/>
         <return-value transfer-ownership="none">
           <doc xml:space="preserve" filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.c">%TRUE if the media can be shared between clients.</doc>
@@ -5281,7 +5305,9 @@ One or more GstRTSPStream objects should be created from the result
 with gst_rtsp_media_create_stream ().
 
 After the media is constructed, it can be configured and then prepared
-with gst_rtsp_media_prepare ().</doc>
+with gst_rtsp_media_prepare ().
+
+The returned media will be locked and must be unlocked afterwards.</doc>
         <source-position filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media-factory.h"/>
         <return-value transfer-ownership="full" nullable="1">
           <doc xml:space="preserve" filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media-factory.c">a new #GstRTSPMedia if the media could be prepared.</doc>
@@ -5430,7 +5456,9 @@ One or more GstRTSPStream objects should be created from the result
 with gst_rtsp_media_create_stream ().
 
 After the media is constructed, it can be configured and then prepared
-with gst_rtsp_media_prepare ().</doc>
+with gst_rtsp_media_prepare ().
+
+The returned media will be locked and must be unlocked afterwards.</doc>
         <source-position filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media-factory.h"/>
         <return-value transfer-ownership="full" nullable="1">
           <doc xml:space="preserve" filename="../subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media-factory.c">a new #GstRTSPMedia if the media could be prepared.</doc>

subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-client.c

@@ -1053,6 +1053,7 @@ find_media (GstRTSPClient * client, GstRTSPContext * ctx, gchar * path,
   } else {
     /* we have seen this path before, used cached media */
     media = priv->media;
+    gst_rtsp_media_lock (media);
     ctx->media = media;
     GST_INFO ("reusing cached media %p for path %s", media, priv->path);
   }
@@ -1104,6 +1105,7 @@ no_thread:
     GST_ERROR ("client %p: can't create thread", client);
     send_generic_error_response (client, GST_RTSP_STS_SERVICE_UNAVAILABLE, ctx);
     gst_rtsp_url_free (url);
+    gst_rtsp_media_unlock (media);
     g_object_unref (media);
     ctx->media = NULL;
     g_object_unref (factory);
@@ -1115,6 +1117,7 @@ no_prepare:
     GST_ERROR ("client %p: can't prepare media", client);
     send_generic_error_response (client, GST_RTSP_STS_SERVICE_UNAVAILABLE, ctx);
     gst_rtsp_url_free (url);
+    gst_rtsp_media_unlock (media);
     g_object_unref (media);
     ctx->media = NULL;
     g_object_unref (factory);
@@ -2852,7 +2855,6 @@ handle_setup_request (GstRTSPClient * client, GstRTSPContext * ctx)
     media = find_media (client, ctx, path, &matched);
     /* need to suspend the media, if the protocol has changed */
     if (media != NULL) {
-      gst_rtsp_media_lock (media);
       gst_rtsp_media_suspend (media);
     }
   } else {
@@ -3340,8 +3342,6 @@ handle_describe_request (GstRTSPClient * client, GstRTSPContext * ctx)
   if (!(media = find_media (client, ctx, path, NULL)))
     goto no_media;
 
-  gst_rtsp_media_lock (media);
-
   if (!(gst_rtsp_media_get_transport_mode (media) &
           GST_RTSP_TRANSPORT_MODE_PLAY))
     goto unsupported_mode;
@@ -3525,7 +3525,6 @@ handle_announce_request (GstRTSPClient * client, GstRTSPContext * ctx)
     goto no_media;
 
   ctx->media = media;
-  gst_rtsp_media_lock (media);
 
   g_signal_emit (client, gst_rtsp_client_signals[SIGNAL_PRE_ANNOUNCE_REQUEST],
       0, ctx, &sig_result);

subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media-factory.c

@@ -1375,6 +1375,8 @@ weak_ref_free (GWeakRef * ref)
  * After the media is constructed, it can be configured and then prepared
  * with gst_rtsp_media_prepare ().
  *
+ * The returned media will be locked and must be unlocked afterwards.
+ *
  * Returns: (transfer full) (nullable): a new #GstRTSPMedia if the media could be prepared.
  */
 GstRTSPMedia *
@@ -1403,12 +1405,33 @@ gst_rtsp_media_factory_construct (GstRTSPMediaFactory * factory,
   if (key) {
     /* we have a key, see if we find a cached media */
     media = g_hash_table_lookup (priv->medias, key);
-    if (media)
+    if (media) {
       g_object_ref (media);
-  } else
-    media = NULL;
+      g_mutex_unlock (&priv->medias_lock);
+
+      /* now need to check if the media is curently in the process of being
+       * unprepared. That always happens while the lock is taken, so take it
+       * here now and then check if we can really use the media */
+      gst_rtsp_media_lock (media);
+      if (!gst_rtsp_media_can_be_shared (media)) {
+        gst_rtsp_media_unlock (media);
+        g_object_unref (media);
+        media = NULL;
+      }
+
+      if (media) {
+        if (key)
+          g_free (key);
+
+        GST_INFO ("reusing cached media %p for url %s", media, url->abspath);
+
+        return media;
+      }
+
+      g_mutex_lock (&priv->medias_lock);
+    }
+  }
 
-  if (media == NULL) {
   /* nothing cached found, try to create one */
   if (klass->construct) {
     media = klass->construct (factory, url);
@@ -1416,17 +1439,17 @@ gst_rtsp_media_factory_construct (GstRTSPMediaFactory * factory,
       g_signal_emit (factory,
           gst_rtsp_media_factory_signals[SIGNAL_MEDIA_CONSTRUCTED], 0, media,
           NULL);
-    } else
-      media = NULL;
+  }
 
   if (media) {
+    gst_rtsp_media_lock (media);
+
     /* configure the media */
     if (klass->configure)
       klass->configure (factory, media);
 
     g_signal_emit (factory,
-          gst_rtsp_media_factory_signals[SIGNAL_MEDIA_CONFIGURE], 0, media,
-          NULL);
+        gst_rtsp_media_factory_signals[SIGNAL_MEDIA_CONFIGURE], 0, media, NULL);
 
     /* check if we can cache this media */
     if (gst_rtsp_media_is_shared (media) && key) {
@@ -1443,7 +1466,6 @@ gst_rtsp_media_factory_construct (GstRTSPMediaFactory * factory,
           (GClosureNotify) weak_ref_free, 0);
     }
   }
-  }
   g_mutex_unlock (&priv->medias_lock);
 
   if (key)

subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.c

@@ -1172,7 +1172,12 @@ gst_rtsp_media_set_shared (GstRTSPMedia * media, gboolean shared)
  * gst_rtsp_media_is_shared:
  * @media: a #GstRTSPMedia
  *
- * Check if the pipeline for @media can be shared between multiple clients.
+ * Check if the pipeline for @media can be shared between multiple clients in
+ * theory. This simply returns the value set via gst_rtsp_media_set_shared().
+ *
+ * To know if a media can be shared in practice, i.e. if it's shareable and
+ * either reusable or was never unprepared before, use
+ * gst_rtsp_media_can_be_shared().
  *
  * Returns: %TRUE if the media can be shared between clients.
  */
@@ -1193,6 +1198,39 @@ gst_rtsp_media_is_shared (GstRTSPMedia * media)
   return res;
 }
 
+/**
+ * gst_rtsp_media_can_be_shared:
+ * @media: a #GstRTSPMedia
+ *
+ * Check if the pipeline for @media can be shared between multiple clients.
+ *
+ * This checks if the media is shareable and whether it is either reusable or
+ * was never unprepared before.
+ *
+ * The function must be called with gst_rtsp_media_lock().
+ *
+ * Returns: %TRUE if the media can be shared between clients.
+ *
+ * Since: 1.24
+ */
+gboolean
+gst_rtsp_media_can_be_shared (GstRTSPMedia * media)
+{
+  GstRTSPMediaPrivate *priv;
+  gboolean res;
+
+  g_return_val_if_fail (GST_IS_RTSP_MEDIA (media), FALSE);
+
+  priv = media->priv;
+
+  g_mutex_lock (&priv->lock);
+  res = priv->shared && (priv->reusable || !priv->reused);
+  g_mutex_unlock (&priv->lock);
+
+  return res;
+}
+
+
 /**
  * gst_rtsp_media_set_reusable:
  * @media: a #GstRTSPMedia

subprojects/gst-rtsp-server/gst/rtsp-server/rtsp-media.h

@@ -221,6 +221,9 @@ void                  gst_rtsp_media_set_shared       (GstRTSPMedia *media, gboo
 GST_RTSP_SERVER_API
 gboolean              gst_rtsp_media_is_shared        (GstRTSPMedia *media);
 
+GST_RTSP_SERVER_API
+gboolean              gst_rtsp_media_can_be_shared    (GstRTSPMedia *media);
+
 GST_RTSP_SERVER_API
 void                  gst_rtsp_media_set_stop_on_disconnect (GstRTSPMedia *media, gboolean stop_on_disconnect);
 

subprojects/gst-rtsp-server/tests/check/gst/media.c

@@ -128,6 +128,7 @@ GST_START_TEST (test_media_seek)
   gst_rtsp_range_free (range);
 
   fail_unless (gst_rtsp_media_unprepare (media));
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   gst_rtsp_url_free (url);
@@ -210,6 +211,7 @@ media_playback_seek_one_active_stream (const gchar * launch_line)
   g_free (range_str);
 
   fail_unless (gst_rtsp_media_unprepare (media));
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   gst_rtsp_url_free (url);
@@ -307,6 +309,7 @@ GST_START_TEST (test_media_seek_no_sinks)
   fail_if (gst_rtsp_media_seek (media, range));
 
   gst_rtsp_range_free (range);
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   gst_rtsp_url_free (url);
@@ -385,6 +388,7 @@ test_prepare_reusable (const gchar * launch_line, gboolean is_live)
   fail_unless (media_has_sdp (media));
   fail_unless (gst_rtsp_media_unprepare (media));
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
   gst_rtsp_url_free (url);
   g_object_unref (factory);
@@ -439,6 +443,7 @@ GST_START_TEST (test_media_prepare)
       GST_RTSP_THREAD_TYPE_MEDIA, NULL);
   fail_if (gst_rtsp_media_prepare (media, thread));
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
   gst_rtsp_url_free (url);
   g_object_unref (factory);
@@ -606,6 +611,7 @@ GST_START_TEST (test_media_shared_race_test_unsuspend_vs_set_state_null)
   g_cond_clear (&sync_cond);
   g_mutex_clear (&sync_mutex);
   fail_unless (gst_rtsp_media_unprepare (media));
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
   gst_rtsp_url_free (url);
   g_object_unref (factory);
@@ -727,6 +733,7 @@ GST_START_TEST (test_media_take_pipeline)
   pipeline = gst_pipeline_new ("media-pipeline");
   gst_rtsp_media_take_pipeline (media, GST_PIPELINE_CAST (pipeline));
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
   gst_rtsp_url_free (url);
   g_object_unref (factory);
@@ -761,6 +768,7 @@ GST_START_TEST (test_media_reset)
   fail_unless_equals_int64 (gst_rtsp_media_seekable (media), G_MAXINT64);
   fail_unless (gst_rtsp_media_suspend (media));
   fail_unless (gst_rtsp_media_unprepare (media));
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   media = gst_rtsp_media_factory_construct (factory, url);
@@ -774,6 +782,7 @@ GST_START_TEST (test_media_reset)
   fail_unless_equals_int64 (gst_rtsp_media_seekable (media), G_MAXINT64);
   fail_unless (gst_rtsp_media_suspend (media));
   fail_unless (gst_rtsp_media_unprepare (media));
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   gst_rtsp_url_free (url);
@@ -914,6 +923,7 @@ GST_START_TEST (test_media_pipeline_error)
   ck_assert (!gst_rtsp_media_prepare (media, thread));
   ck_assert_uint_eq (handled_messages, 1);
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
   gst_rtsp_url_free (url);
   g_object_unref (factory);

subprojects/gst-rtsp-server/tests/check/gst/mediafactory.c

@@ -81,10 +81,12 @@ GST_START_TEST (test_launch_construct)
 
   media = gst_rtsp_media_factory_construct (factory, url);
   fail_unless (GST_IS_RTSP_MEDIA (media));
+  gst_rtsp_media_unlock (media);
 
   media2 = gst_rtsp_media_factory_construct (factory, url);
   fail_unless (GST_IS_RTSP_MEDIA (media2));
   fail_if (media == media2);
+  gst_rtsp_media_unlock (media2);
 
   g_object_unref (media);
   g_object_unref (media2);
@@ -119,10 +121,12 @@ GST_START_TEST (test_shared)
 
   media = gst_rtsp_media_factory_construct (factory, url);
   fail_unless (GST_IS_RTSP_MEDIA (media));
+  gst_rtsp_media_unlock (media);
 
   media2 = gst_rtsp_media_factory_construct (factory, url);
   fail_unless (GST_IS_RTSP_MEDIA (media2));
   fail_unless (media == media2);
+  gst_rtsp_media_unlock (media2);
 
   g_object_unref (media);
   g_object_unref (media2);
@@ -200,7 +204,7 @@ GST_START_TEST (test_addresspool)
   addr = gst_rtsp_stream_get_multicast_address (stream, G_SOCKET_FAMILY_IPV4);
   fail_unless (addr == NULL);
 
-
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   g_object_unref (pool);
@@ -272,6 +276,7 @@ GST_START_TEST (test_permissions)
   fail_if (gst_rtsp_permissions_is_allowed (perms, "missing",
           "media.factory.access"));
   gst_rtsp_permissions_unref (perms);
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   gst_rtsp_url_free (url);
@@ -297,6 +302,7 @@ GST_START_TEST (test_reset)
   fail_unless (GST_IS_RTSP_MEDIA (media));
   fail_if (gst_rtsp_media_get_suspend_mode (media) !=
       GST_RTSP_SUSPEND_MODE_NONE);
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   gst_rtsp_media_factory_set_suspend_mode (factory,
@@ -306,6 +312,7 @@ GST_START_TEST (test_reset)
   fail_unless (GST_IS_RTSP_MEDIA (media));
   fail_if (gst_rtsp_media_get_suspend_mode (media) !=
       GST_RTSP_SUSPEND_MODE_RESET);
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   gst_rtsp_url_free (url);
@@ -363,6 +370,7 @@ GST_START_TEST (test_mcast_ttl)
   fail_unless (stream != NULL);
   fail_unless (gst_rtsp_stream_get_max_mcast_ttl (stream) == 3);
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
 
   gst_rtsp_url_free (url);
@@ -413,6 +421,7 @@ GST_START_TEST (test_allow_bind_mcast)
   fail_unless (stream != NULL);
   fail_unless (gst_rtsp_stream_is_bind_mcast_address (stream) == TRUE);
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (media);
   gst_rtsp_url_free (url);
   g_object_unref (factory);

subprojects/gst-rtsp-server/tests/check/gst/sessionmedia.c

@@ -97,6 +97,7 @@ GST_START_TEST (test_setup_url)
   gst_rtsp_url_free (setup_url);
   gst_rtsp_url_free (url);
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (sm);
 
   g_object_unref (factory);
@@ -161,6 +162,7 @@ GST_START_TEST (test_rtsp_state)
 
   gst_rtsp_url_free (url);
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (sm);
 
   g_object_unref (factory);
@@ -231,6 +233,7 @@ GST_START_TEST (test_transports)
 
   gst_rtsp_url_free (url);
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (sm);
 
   g_object_unref (factory);
@@ -317,6 +320,7 @@ GST_START_TEST (test_time_and_rtpinfo)
   gst_rtsp_url_free (setup_url);
   gst_rtsp_url_free (url);
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (sm);
 
   g_object_unref (factory);
@@ -372,6 +376,7 @@ GST_START_TEST (test_allocate_channels)
 
   gst_rtsp_url_free (url);
 
+  gst_rtsp_media_unlock (media);
   g_object_unref (sm);
 
   g_object_unref (factory);