From d819f6913c900fd07e240f5f307cbff23763f1dd Mon Sep 17 00:00:00 2001 From: Havard Graff Date: Sun, 28 Oct 2018 12:46:09 +0100 Subject: [PATCH] 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. --- gst/gstpad.c | 16 +++---- tests/check/gst/gstpad.c | 91 +++++++++++++++++++--------------------- 2 files changed, 52 insertions(+), 55 deletions(-) diff --git a/gst/gstpad.c b/gst/gstpad.c index da9f866a4f..e064c0c0d9 100644 --- a/gst/gstpad.c +++ b/gst/gstpad.c @@ -172,7 +172,7 @@ typedef struct gboolean handled; gboolean marshalled; - GHook **called_probes; + gulong *called_probes; guint n_called_probes; guint called_probes_size; gboolean retry; @@ -3472,7 +3472,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; @@ -3485,17 +3485,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; @@ -3691,7 +3691,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 b9def0f1f9..6b3d82f9f3 100644 --- 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) +probe_block_remove (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) -{ - 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 (); + + /* 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); + + /* wait for the callback */ + while (!called) + g_thread_yield (); + + 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 (); } - probe_b = gst_pad_add_probe (pad, + called = FALSE; + gst_pad_add_probe (pad, GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_BUFFER, - probe_block_b, &probe_b_called, NULL); + probe_block_remove, &called, NULL); gst_pad_remove_probe (pad, probe_a); /* wait for the callback */ - while (!probe_b_called) { - g_usleep (10000); - } - - /* wait for the block */ - while (!gst_pad_is_blocking (pad)) { - g_usleep (10000); - } - - gst_pad_add_probe (pad, - GST_PAD_PROBE_TYPE_BLOCK | GST_PAD_PROBE_TYPE_BUFFER, - probe_block_c, &probe_c_called, NULL); - - gst_pad_remove_probe (pad, probe_b); - - /* 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); }