gststructure: Fix gst_structure_take ownership handling

The old code would leave a dangling pointer in oldstr_ptr if two threads
attempted to take the same structure into the same location at the same
time:

1. First "oldstr == newstr" check (before the loop) fails.
2. Compare-and-exchange fails, due to a second thread completing the
   same gst_structure_take.
3. Second "oldstr == newstr" check (in the loop) succeeds, loop breaks.
4. "oldstr" check succeeds, old structure gets freed.
5. oldstr_ptr now contains a dangling pointer.

This shouldn't happen in code that handles ownership sanely, so check
that we don't try to do this and complain loudly.

Also simplify the function by using a do-while loop, like
gst_mini_object_take.

https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/413
This commit is contained in:
Jan Alexander Steffens (heftig) 2020-03-23 12:28:12 +01:00 committed by GStreamer Merge Bot
parent e45f187d13
commit a20ff6aaf4

View file

@ -511,12 +511,15 @@ gst_clear_structure (GstStructure ** structure_ptr)
* a #GstStructure to take
* @newstr: (transfer full) (allow-none): a new #GstStructure
*
* Atomically modifies a pointer to point to a new object.
* Atomically modifies a pointer to point to a new structure.
* The #GstStructure @oldstr_ptr is pointing to is freed and
* @newstr is taken ownership over.
*
* Either @newstr and the value pointed to by @oldstr_ptr may be %NULL.
*
* It is a programming error if both @newstr and the value pointed to by
* @oldstr_ptr refer to the same, non-%NULL structure.
*
* Returns: %TRUE if @newstr was different from @oldstr_ptr
*
* Since: 1.18
@ -528,22 +531,19 @@ gst_structure_take (GstStructure ** oldstr_ptr, GstStructure * newstr)
g_return_val_if_fail (oldstr_ptr != NULL, FALSE);
oldstr = g_atomic_pointer_get ((gpointer *) oldstr_ptr);
if (G_UNLIKELY (oldstr == newstr))
return FALSE;
while (G_UNLIKELY (!g_atomic_pointer_compare_and_exchange ((gpointer *)
oldstr_ptr, oldstr, newstr))) {
do {
oldstr = g_atomic_pointer_get ((gpointer *) oldstr_ptr);
if (G_UNLIKELY (oldstr == newstr))
break;
}
if (G_UNLIKELY (oldstr == newstr)) {
g_return_val_if_fail (newstr == NULL, FALSE);
return FALSE;
}
} while (G_UNLIKELY (!g_atomic_pointer_compare_and_exchange ((gpointer *)
oldstr_ptr, oldstr, newstr)));
if (oldstr)
gst_structure_free (oldstr);
return oldstr != newstr;
return TRUE;
}
/**