gst: Delay creation of threadpools
authorEdward Hervey <edward@centricular.com>
Fri, 12 Jun 2020 13:07:42 +0000 (15:07 +0200)
committerEdward Hervey <bilboed@bilboed.com>
Tue, 16 Jun 2020 06:23:21 +0000 (08:23 +0200)
Since glib 2.64, gthreadpool will start waiting on a GCond immediately upon
creation. This can cause issues if we fork *before* actually using the
threadpool since we will then be signalling that GCond ... from another process
and that will never work.

Instead, delay creationg of thread pools until the very first time we need
them. This introduces a minor (un-noticeable) delay when needing a new thread
but fixes the issues for all users of GSTreamer that will call gst_init, then
fork and actually start pipelines.

See https://gitlab.gnome.org/GNOME/glib/-/issues/2131 for more context.

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

gst/gstelement.c
gst/gsttask.c

index 5f71cb7..de77815 100644 (file)
@@ -159,6 +159,7 @@ static void gst_element_call_async_func (gpointer data, gpointer user_data);
 static GstObjectClass *parent_class = NULL;
 static guint gst_element_signals[LAST_SIGNAL] = { 0 };
 
+static GMutex _element_pool_lock;
 static GThreadPool *gst_element_pool = NULL;
 
 /* this is used in gstelementfactory.c:gst_element_register() */
@@ -194,19 +195,22 @@ gst_element_get_type (void)
   return gst_element_type;
 }
 
-static void
+static GThreadPool *
 gst_element_setup_thread_pool (void)
 {
   GError *err = NULL;
+  GThreadPool *pool;
 
   GST_DEBUG ("creating element thread pool");
-  gst_element_pool =
+  pool =
       g_thread_pool_new ((GFunc) gst_element_call_async_func, NULL, -1, FALSE,
       &err);
   if (err != NULL) {
     g_critical ("could not alloc threadpool %s", err->message);
     g_clear_error (&err);
   }
+
+  return pool;
 }
 
 static void
@@ -273,8 +277,6 @@ gst_element_class_init (GstElementClass * klass)
   klass->set_context = GST_DEBUG_FUNCPTR (gst_element_set_context_default);
 
   klass->elementfactory = NULL;
-
-  gst_element_setup_thread_pool ();
 }
 
 static void
@@ -3809,16 +3811,22 @@ gst_element_call_async (GstElement * element, GstElementCallAsyncFunc func,
   async_data->user_data = user_data;
   async_data->destroy_notify = destroy_notify;
 
-  g_thread_pool_push (gst_element_pool, async_data, NULL);
+  g_mutex_lock (&_element_pool_lock);
+  if (G_UNLIKELY (gst_element_pool == NULL))
+    gst_element_pool = gst_element_setup_thread_pool ();
+  g_thread_pool_push ((GThreadPool *) gst_element_pool, async_data, NULL);
+  g_mutex_unlock (&_element_pool_lock);
 }
 
 void
 _priv_gst_element_cleanup (void)
 {
+  g_mutex_lock (&_element_pool_lock);
   if (gst_element_pool) {
-    g_thread_pool_free (gst_element_pool, FALSE, TRUE);
-    gst_element_setup_thread_pool ();
+    g_thread_pool_free ((GThreadPool *) gst_element_pool, FALSE, TRUE);
+    gst_element_pool = NULL;
   }
+  g_mutex_unlock (&_element_pool_lock);
 }
 
 /**
index 5e057a2..6c022fb 100644 (file)
@@ -145,6 +145,8 @@ static void gst_task_func (GstTask * task);
 
 static GMutex pool_lock;
 
+static GstTaskPool *_global_task_pool = NULL;
+
 #define _do_init \
 { \
   GST_DEBUG_CATEGORY_INIT (task_debug, "task", 0, "Processing tasks"); \
@@ -153,19 +155,18 @@ static GMutex pool_lock;
 G_DEFINE_TYPE_WITH_CODE (GstTask, gst_task, GST_TYPE_OBJECT,
     G_ADD_PRIVATE (GstTask) _do_init);
 
+/* Called with pool_lock */
 static void
-init_klass_pool (GstTaskClass * klass)
+ensure_klass_pool (GstTaskClass * klass)
 {
-  g_mutex_lock (&pool_lock);
-  if (klass->pool) {
-    gst_task_pool_cleanup (klass->pool);
-    gst_object_unref (klass->pool);
+  if (G_UNLIKELY (_global_task_pool == NULL)) {
+    _global_task_pool = gst_task_pool_new ();
+    gst_task_pool_prepare (_global_task_pool, NULL);
+
+    /* Classes are never destroyed so this ref will never be dropped */
+    GST_OBJECT_FLAG_SET (_global_task_pool, GST_OBJECT_FLAG_MAY_BE_LEAKED);
   }
-  klass->pool = gst_task_pool_new ();
-  /* Classes are never destroyed so this ref will never be dropped */
-  GST_OBJECT_FLAG_SET (klass->pool, GST_OBJECT_FLAG_MAY_BE_LEAKED);
-  gst_task_pool_prepare (klass->pool, NULL);
-  g_mutex_unlock (&pool_lock);
+  klass->pool = _global_task_pool;
 }
 
 static void
@@ -176,8 +177,6 @@ gst_task_class_init (GstTaskClass * klass)
   gobject_class = (GObjectClass *) klass;
 
   gobject_class->finalize = gst_task_finalize;
-
-  init_klass_pool (klass);
 }
 
 static void
@@ -197,6 +196,7 @@ gst_task_init (GstTask * task)
   /* use the default klass pool for this task, users can
    * override this later */
   g_mutex_lock (&pool_lock);
+  ensure_klass_pool (klass);
   task->priv->pool = gst_object_ref (klass->pool);
   g_mutex_unlock (&pool_lock);
 }
@@ -378,7 +378,14 @@ gst_task_cleanup_all (void)
   GstTaskClass *klass;
 
   if ((klass = g_type_class_peek (GST_TYPE_TASK))) {
-    init_klass_pool (klass);
+    if (klass->pool) {
+      g_mutex_lock (&pool_lock);
+      gst_task_pool_cleanup (klass->pool);
+      gst_object_unref (klass->pool);
+      klass->pool = NULL;
+      _global_task_pool = NULL;
+      g_mutex_unlock (&pool_lock);
+    }
   }
 
   /* GstElement owns a GThreadPool */