gstpad: use hook_id instead of hook in called_probes list

A pointer to a hook in this list can easily not be unique, given both
the slice-allocator reusing memory, and the OS re-using freed blocks
in malloc.

By doing many repeated add and remove of probes, this becomes very easily
reproduced.

Instead use hook_id, which *is* unique for a added GHook.

diff --git a/gst/gstpad.c b/gst/gstpad.c
index 3e177f2..b50e51f 100644 (file)
--- a/gst/gstpad.c
+++ b/gst/gstpad.c
@@ -169,7 +169,7 @@ typedef struct
   gboolean handled;
   gboolean marshalled;
 
-  GHook **called_probes;
+  gulong *called_probes;
   guint n_called_probes;
   guint called_probes_size;
   gboolean retry;
@@ -3468,7 +3468,7 @@ probe_hook_marshal (GHook * hook, ProbeMarshall * data)
    * if we're actually calling probes a second time */
   if (data->retry) {
     for (i = 0; i < data->n_called_probes; i++) {
-      if (data->called_probes[i] == hook) {
+      if (data->called_probes[i] == hook->hook_id) {
         GST_CAT_LOG_OBJECT (GST_CAT_SCHEDULING, pad,
             "hook %lu already called", hook->hook_id);
         return;
@@ -3481,17 +3481,17 @@ probe_hook_marshal (GHook * hook, ProbeMarshall * data)
     if (data->called_probes_size > N_STACK_ALLOCATE_PROBES) {
       data->called_probes_size *= 2;
       data->called_probes =
-          g_renew (GHook *, data->called_probes, data->called_probes_size);
+          g_renew (gulong, data->called_probes, data->called_probes_size);
     } else {
-      GHook **tmp = data->called_probes;
+      gulong *tmp = data->called_probes;
 
       data->called_probes_size *= 2;
-      data->called_probes = g_new (GHook *, data->called_probes_size);
+      data->called_probes = g_new (gulong, data->called_probes_size);
       memcpy (data->called_probes, tmp,
-          N_STACK_ALLOCATE_PROBES * sizeof (GHook *));
+          N_STACK_ALLOCATE_PROBES * sizeof (gulong));
     }
   }
-  data->called_probes[data->n_called_probes++] = hook;
+  data->called_probes[data->n_called_probes++] = hook->hook_id;
 
   flags = hook->flags >> G_HOOK_FLAG_USER_SHIFT;
   type = info->type;
@@ -3687,7 +3687,7 @@ do_probe_callbacks (GstPad * pad, GstPadProbeInfo * info,
   ProbeMarshall data;
   guint cookie;
   gboolean is_block;
-  GHook *called_probes[N_STACK_ALLOCATE_PROBES];
+  gulong called_probes[N_STACK_ALLOCATE_PROBES];
 
   data.pad = pad;
   data.info = info;

diff --git a/tests/check/gst/gstpad.c b/tests/check/gst/gstpad.c
index 9968db3..5ac6c4e 100644 (file)
--- a/tests/check/gst/gstpad.c
+++ b/tests/check/gst/gstpad.c
@@ -1812,28 +1812,19 @@ GST_START_TEST (test_pad_probe_block_and_drop_buffer)
 GST_END_TEST;
 
 static GstPadProbeReturn
-probe_block_a (GstPad * pad, GstPadProbeInfo * info, gpointer user_data)
+probe_block_ok (GstPad * pad, GstPadProbeInfo * info, gpointer user_data)
 {
+  gboolean *called = user_data;
+  if (called)
+    *called = TRUE;
   return GST_PAD_PROBE_OK;
 }
 
 static GstPadProbeReturn
-probe_block_b (GstPad * pad, GstPadProbeInfo * info, gpointer user_data)
-{
-  gboolean *probe_b_called = user_data;
-
-  *probe_b_called = TRUE;
-
-  return GST_PAD_PROBE_OK;
-}
-
-static GstPadProbeReturn
-probe_block_c (GstPad * pad, GstPadProbeInfo * info, gpointer user_data)
+probe_block_remove (GstPad * pad, GstPadProbeInfo * info, gpointer user_data)
 {
-  gboolean *probe_c_called = user_data;
-
-  *probe_c_called = TRUE;
-
+  gboolean *called = user_data;
+  *called = TRUE;
   return GST_PAD_PROBE_REMOVE;
 }
 
@@ -1842,8 +1833,8 @@ GST_START_TEST (test_pad_probe_block_add_remove)
   GstPad *pad;
   GThread *thread;
   gulong probe_a, probe_b;
-  gboolean probe_b_called = FALSE;
-  gboolean probe_c_called = FALSE;
+  gboolean called;
+  guint r;
 
   pad = gst_pad_new ("src", GST_PAD_SRC);
   fail_unless (pad != NULL);
@@ -1859,7 +1850,7 @@ GST_START_TEST (test_pad_probe_block_add_remove)
 
   probe_a = gst_pad_add_probe (pad,
       GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_BUFFER,
-      probe_block_a, NULL, NULL);
+      probe_block_ok, NULL, NULL);
 
   fail_unless (pad->num_probes == 1);
   fail_unless (pad->num_blocked == 1);
@@ -1867,45 +1858,51 @@ GST_START_TEST (test_pad_probe_block_add_remove)
   thread = g_thread_try_new ("gst-check", (GThreadFunc) push_buffer_async,
       pad, NULL);
 
-  /* wait for the block */
-  while (!gst_pad_is_blocking (pad)) {
-    g_usleep (10000);
-  }
+    /* wait for the block */
+  while (!gst_pad_is_blocking (pad))
+    g_thread_yield ();
 
-  probe_b = gst_pad_add_probe (pad,
-      GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_BUFFER,
-      probe_block_b, &probe_b_called, NULL);
+  /* alternate 2 probes 100 times */
+  for (r = 0; r < 100; r++) {
+    called = FALSE;
+    probe_b = gst_pad_add_probe (pad,
+        GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_BUFFER,
+        probe_block_ok, &called, NULL);
 
-  gst_pad_remove_probe (pad, probe_a);
+    gst_pad_remove_probe (pad, probe_a);
 
-  /* wait for the callback */
-  while (!probe_b_called) {
-    g_usleep (10000);
-  }
+    /* wait for the callback */
+    while (!called)
+      g_thread_yield ();
 
-  /* wait for the block */
-  while (!gst_pad_is_blocking (pad)) {
-    g_usleep (10000);
+    called = FALSE;
+    probe_a = gst_pad_add_probe (pad,
+        GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_BUFFER,
+        probe_block_ok, &called, NULL);
+
+    gst_pad_remove_probe (pad, probe_b);
+
+    /* wait for the callback */
+    while (!called)
+      g_thread_yield ();
   }
 
+  called = FALSE;
   gst_pad_add_probe (pad,
       GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_BUFFER,
-      probe_block_c, &probe_c_called, NULL);
+      probe_block_remove, &called, NULL);
 
-  gst_pad_remove_probe (pad, probe_b);
+  gst_pad_remove_probe (pad, probe_a);
 
   /* wait for the callback */
-  while (!probe_c_called) {
-    g_usleep (10000);
-  }
+  while (!called)
+    g_thread_yield ();
 
   /* wait for the unblock */
-  while (gst_pad_is_blocking (pad)) {
-    g_usleep (10000);
-  }
+  while (gst_pad_is_blocking (pad))
+    g_thread_yield ();
 
   gst_object_unref (pad);
-
   g_thread_join (thread);
 }