Created attachment 371870 [details] [review]
WIP: pad: Fix race condition causing the same probe to be called multiple times

Probes were remembering a cookie that was used to check if the probe was
already called this time before the probes list changed. However the
same probes could've been called by another thread in between and thus
gotten a new cookie, and would then be called a second time.

FIXME: We remember all called probes in a GSList for now, causing lots
of new allocations. An allocation-free solution should be found.

Maybe we can do something with an g_alloca()-ed array? Keep track how much space we allocated, and if we exceed it just do a new g_alloca() copy old array over and continue. There's surely a limit to the number of probes on a pad in practice?

(In reply to Tim-Philipp Müller from comment #2)
> Maybe we can do something with an g_alloca()-ed array? Keep track how much
> space we allocated, and if we exceed it just do a new g_alloca() copy old
> array over and continue. There's surely a limit to the number of probes on a
> pad in practice?

Yes except that if you use g_alloca() inside a loop, the allocation will be gone every iteration (we have a "goto again" loop here).

I could allocate a big enough array with g_alloca (16 items is surely enough?), and if that ever happens to be not enough re-allocate with g_malloc

Created attachment 371881 [details] [review]
pad: Fix race condition causing the same probe to be called multiple times

Probes were remembering a cookie that was used to check if the probe was
already called this time before the probes list changed. However the
same probes could've been called by another thread in between and thus
gotten a new cookie, and would then be called a second time.

Review of attachment 371881 [details] [review]:

::: gst/gstpad.c
@@ +3475,2 @@
   /* if we have called this callback, do nothing */
+  for (i = 0; i < data->n_called_probes; i++) {

This loop is quadratic in number of probes... but this shouldn't really matter in practice.

@@ +3487,3 @@
+      data->called_probes_size *= 2;
+      data->called_probes =
+          g_renew (GHook *, data->called_probes, data->called_probes_size);

realloc here because it's potentially more efficient than the code path below with memcpy

Created attachment 371941 [details] [review]
pad: Fix race condition causing the same probe to be called multiple times

Probes were remembering a cookie that was used to check if the probe was
already called this time before the probes list changed. However the
same probes could've been called by another thread in between and thus
gotten a new cookie, and would then be called a second time.

Attachment 371941 [details] pushed as 13d5957 - pad: Fix race condition causing the same probe to be called multiple times