pad: increase ref-count of hook before exit critical section to perform probe callback

Probe callback is called not holding the mutex, so the probes GHookList is is corrupted if g_hook_destroy_link over the same hook is called multiple times (inside cleanup_hook) due to several reasons:

• While the callback is in process the probe is removed (gst_pad_remove_probe) and the callback returns GST_PAD_PROBE_REMOVE.
• The callback is called multiple times concurrently and returns GST_PAD_PROBE_REMOVE.
• There might be other cases.

To support these cases we must ensure the hook ref is valid so g_hook_destroy_link can be called multiple times, ensuring the probes GHookist remains congruent and so the the memory is not corrupted.

This is a follow up of !798 (merged), which drastically reduced issues regarding GHookList destroying.

Anyway we continued having some issues. Specifically:

• g-warnings:

g_hook_ref: assertion 'hook->ref_count > 0' failed
g_hook_unref: assertion 'hook->ref_count > 0' failed

• SIGSEGs like:

Thread 59 (Thread 0x7f45ff7fe700 (LWP 22655)):
#0  g_hook_ref (hook_list=0x7f4530033008, hook=0xc17f3e9b) at ghook.c:422
#1  0x00007f4683dfaac3 in g_hook_list_clear (hook_list=hook_list@entry=0x7f4530033008) at ghook.c:245
#2  0x00007f46843cd009 in gst_pad_dispose (object=0x7f4530032f70) at gstpad.c:721
#3  0x00007f46841187b5 in g_object_unref (_object=0x7f4530032f70) at gobject.c:3309

• Forever loop into g_hook_list_clear

These issues show that the memory regarding GHookList is corrupted and as the usage that GstPad does is easy I was able to create the next simple test simulating how the memory can be corrupted by the GstPad:

typedef struct
{
  GHook hook;
  guint cookie;
} GstProbe;

GST_START_TEST(hooks)
{
  GHookList probes;
  GHook *hook;

  g_hook_list_init(&probes, sizeof(GstProbe));

  hook = g_hook_alloc(&probes);
  g_hook_append(&probes, hook);

  /* g_hook_list_clear can NOT be called multiple times safety */
  g_hook_destroy_link(&probes, hook);
  g_hook_destroy_link(&probes, hook);

  /* g_hook_list_clear can be called multiple times safety */
  g_hook_list_clear(&probes);
  g_hook_list_clear(&probes);
}

GST_END_TEST;

The test passes, but valgrind shows memory corruption:

==4302== Invalid read of size 4
==4302==    at 0x51D298A: g_hook_destroy_link (ghook.c:321)
==4302==    by 0x109B25: hooks (utils.c:893)
==4302==    by 0x54F37CE: tcase_run_tfun_fork (check_run.c:465)
==4302==    by 0x54F37CE: srunner_iterate_tcase_tfuns (check_run.c:237)
==4302==    by 0x54F37CE: srunner_run_tcase (check_run.c:377)
==4302==    by 0x54F37CE: srunner_iterate_suites (check_run.c:205)
==4302==    by 0x54F37CE: srunner_run_tagged (check_run.c:740)
==4302==    by 0x54E7D9C: gst_check_run_suite (gstcheck.c:1075)
==4302==    by 0x109933: main (utils.c:935)

...

==4302== Invalid read of size 8
==4302==    at 0x51D298E: g_hook_destroy_link (ghook.c:322)
==4302==    by 0x109B25: hooks (utils.c:893)
==4302==    by 0x54F37CE: tcase_run_tfun_fork (check_run.c:465)
==4302==    by 0x54F37CE: srunner_iterate_tcase_tfuns (check_run.c:237)
==4302==    by 0x54F37CE: srunner_run_tcase (check_run.c:377)
==4302==    by 0x54F37CE: srunner_iterate_suites (check_run.c:205)
==4302==    by 0x54F37CE: srunner_run_tagged (check_run.c:740)
==4302==    by 0x54E7D9C: gst_check_run_suite (gstcheck.c:1075)
==4302==    by 0x109933: main (utils.c:935)

To support multiple g_hook_destroy_link we need to maintain a valid hook ref-count.

Hence, next change fixes the issue:

@@ -888,9 +888,14 @@ GST_START_TEST(hooks)
   hook = g_hook_alloc(&probes);
   g_hook_append(&probes, hook);
 
-  /* g_hook_list_clear can NOT be called multiple times safety */
+  /*
+   * If the ref-count of hook remains valid
+   * g_hook_list_clear CAN be called multiple times safety
+   */
+  g_hook_ref(&probes, hook);
   g_hook_destroy_link(&probes, hook);
   g_hook_destroy_link(&probes, hook);
+  g_hook_unref(&probes, hook);
 
   /* g_hook_list_clear can be called multiple times safety */
   g_hook_list_clear(&probes);