<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer] clean-up various leaks"
href="https://bugs.webkit.org/show_bug.cgi?id=154285">bug 154285</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #271425 Flags</td>
<td>review?
</td>
<td>review+
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer] clean-up various leaks"
href="https://bugs.webkit.org/show_bug.cgi?id=154285#c6">Comment # 6</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GStreamer] clean-up various leaks"
href="https://bugs.webkit.org/show_bug.cgi?id=154285">bug 154285</a>
from <span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=271425&action=diff" name="attach_271425" title="patch">attachment 271425</a> <a href="attachment.cgi?id=271425&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=271425&action=review">https://bugs.webkit.org/attachment.cgi?id=271425&action=review</a>
<span class="quote">>>>>> 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.</span >
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().
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:127
> + ASSERT(!ptr || !g_object_is_floating(G_OBJECT(ptr)));</span >
no need to cast to GObject, g_object_is_floating expects a gpointer.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:147
> + ASSERT(!ptr || !g_object_is_floating(G_OBJECT(ptr)));</span >
ditto.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:167
> + ASSERT(!ptr || !g_object_is_floating(G_OBJECT(ptr)));</span >
ditto.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>