validate: Check that pull_range is called from the streaming thread
authorThibault Saunier <tsaunier@igalia.com>
Mon, 15 Jul 2019 16:36:23 +0000 (12:36 -0400)
committerThibault Saunier <tsaunier@gnome.org>
Wed, 2 Oct 2019 19:22:31 +0000 (19:22 +0000)
`gst_pad_pull_range` should always be called from the streaming thread,
we now check that when pull_range is called, and if the sinkpad calling
the function has a GstTask with a running thread, the function is called
from that thread.

validate/gst/validate/gst-validate-pad-monitor.c
validate/gst/validate/gst-validate-pad-monitor.h
validate/gst/validate/gst-validate-report.c
validate/gst/validate/gst-validate-report.h

index 082f8bb..dd8f4c4 100644 (file)
@@ -2468,6 +2468,44 @@ gst_validate_pad_monitor_activatemode_func (GstPad * pad, GstObject * parent,
   return ret;
 }
 
+static GstFlowReturn
+gst_validate_pad_monitor_get_range_func (GstPad * pad, GstObject * parent,
+    guint64 offset, guint length, GstBuffer ** buffer)
+{
+  GstValidatePadMonitor *pad_monitor = _GET_PAD_MONITOR (pad);
+
+  if (pad_monitor->get_range_func) {
+    GstPad *peer = gst_pad_get_peer (pad);
+    GstTask *task = NULL;
+    GThread *thread = NULL;
+
+    if (peer) {
+      GST_OBJECT_LOCK (peer);
+      task = GST_PAD_TASK (peer);
+      if (task) {
+        GST_OBJECT_LOCK (task);
+        /* Only doing pointer comparison, no need to hold a ref */
+        thread = task->thread;
+        GST_OBJECT_UNLOCK (task);
+      }
+      GST_OBJECT_UNLOCK (peer);
+
+      if (thread && thread != g_thread_self ()) {
+        GST_VALIDATE_REPORT (pad_monitor, PULL_RANGE_FROM_WRONG_THREAD,
+            "Pulling from wrong thread, expected pad thread: %p, got %p",
+            task->thread, g_thread_self ());
+      }
+
+      gst_object_unref (peer);
+    }
+
+    return pad_monitor->get_range_func (pad, parent, offset, length, buffer);
+  }
+
+  return GST_FLOW_NOT_SUPPORTED;
+
+}
+
 /* The interval between two buffer frequency checks */
 #define BUF_FREQ_CHECK_INTERVAL (GST_SECOND)
 
@@ -2909,6 +2947,7 @@ gst_validate_pad_monitor_do_setup (GstValidateMonitor * monitor)
   pad_monitor->event_full_func = GST_PAD_EVENTFULLFUNC (pad);
   pad_monitor->query_func = GST_PAD_QUERYFUNC (pad);
   pad_monitor->activatemode_func = GST_PAD_ACTIVATEMODEFUNC (pad);
+  pad_monitor->get_range_func = GST_PAD_GETRANGEFUNC (pad);
   if (GST_PAD_DIRECTION (pad) == GST_PAD_SINK) {
 
     pad_monitor->chain_func = GST_PAD_CHAINFUNC (pad);
@@ -2936,6 +2975,11 @@ gst_validate_pad_monitor_do_setup (GstValidateMonitor * monitor)
   gst_pad_set_activatemode_function (pad,
       gst_validate_pad_monitor_activatemode_func);
 
+  if (GST_PAD_IS_SRC (pad)) {
+    gst_pad_set_getrange_function (pad,
+        gst_validate_pad_monitor_get_range_func);
+  }
+
   gst_validate_reporter_set_name (GST_VALIDATE_REPORTER (monitor),
       g_strdup_printf ("%s:%s", GST_DEBUG_PAD_NAME (pad)));
 
index b2fc6b8..a50d5c6 100644 (file)
@@ -63,6 +63,7 @@ struct _GstValidatePadMonitor {
   GstPadEventFullFunction event_full_func;
   GstPadQueryFunction query_func;
   GstPadActivateModeFunction activatemode_func;
+  GstPadGetRangeFunction get_range_func;
 
   gulong pad_probe_id;
 
index de72824..a4c45fe 100644 (file)
@@ -414,6 +414,10 @@ gst_validate_report_load_issues (void)
   REGISTER_VALIDATE_ISSUE (CRITICAL, G_LOG_CRITICAL,
       "We got a g_log critical issue", NULL);
   REGISTER_VALIDATE_ISSUE (ISSUE, G_LOG_ISSUE, "We got a g_log issue", NULL);
+
+  REGISTER_VALIDATE_ISSUE (CRITICAL, PULL_RANGE_FROM_WRONG_THREAD,
+      "gst_pad_pull_range called from wrong thread",
+      _("gst_pad_pull_range has to be called from the sinkpad task thread."));
 }
 
 gboolean
index 95a672f..098b74b 100644 (file)
@@ -81,6 +81,8 @@ typedef enum {
 #define FLOW_ERROR_WITHOUT_ERROR_MESSAGE         _QUARK("buffer::flow-error-without-error-message")
 #define BUFFER_MISSING_DISCONT                   _QUARK("buffer::missing-discont")
 
+#define PULL_RANGE_FROM_WRONG_THREAD             _QUARK("threading::pull-range-from-wrong-thread")
+
 #define CAPS_IS_MISSING_FIELD                    _QUARK("caps::is-missing-field")
 #define CAPS_FIELD_HAS_BAD_TYPE                  _QUARK("caps::field-has-bad-type")
 #define CAPS_EXPECTED_FIELD_NOT_FOUND            _QUARK("caps::expected-field-not-found")