[Webkit-unassigned] [Bug 154285] [GStreamer] clean-up various leaks

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=154285

Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #271425|review?                     |review+
              Flags|                            |

--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 271425
  --> https://bugs.webkit.org/attachment.cgi?id=271425
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=271425&action=review

>>>>> Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp:196
>>>>> -    priv->task = gst_task_new(reinterpret_cast<GstTaskFunction>(webKitWebAudioSrcLoop), src, 0);
>>>>> +    priv->task = adoptGRef(gst_task_new(reinterpret_cast<GstTaskFunction>(webKitWebAudioSrcLoop), src, 0));
>>>> 
>>>> GstTask is GInitiallyUnowned, so this returns a floating reference that we should consume here, so adoptGRef is not correct. Aren't you hitting the assert in GRefPtr<GstTask> adoptGRef(GstTask* ptr) ?
>>> 
>>> Heh. No :)
>>> But I'll double-check.
>> 
>> This should be adoptGRef(g_object_ref_sink(gst_task_new(...))), correct?
>> 
>> I think floating references are not useful in combination with GRefPtr, so why don't we take it always in adoptGRef() (e.g. by calling g_object_is_floating() and then g_object_ref_sink() if that return TRUE)? What is the value in asserting that the ptr is not floating? (Why do we do that only in GRefPtrGStreamer.cpp and not in GRefPtr.cpp?) Am I missing something here?
> 
> No.

So, the thing is that gst_task_init is consuming its own floating ref. This is soooo confusing. Please, add a comment here explaining why we are adopting the ref returned by gst_task_new().

> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:127
> +    ASSERT(!ptr || !g_object_is_floating(G_OBJECT(ptr)));

no need to cast to GObject, g_object_is_floating expects a gpointer.

> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:147
> +    ASSERT(!ptr || !g_object_is_floating(G_OBJECT(ptr)));

ditto.

> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:167
> +    ASSERT(!ptr || !g_object_is_floating(G_OBJECT(ptr)));

ditto.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160219/37882534/attachment-0001.html>