[Webkit-unassigned] [Bug 16401] [GTK] GObject/C DOM binding

[bugzilla-daemon at webkit.org]

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





------- Comment #84 from mrowe at apple.com  2008-10-17 14:13 PDT -------
(In reply to comment #82)
> > The way it looks, there seems to be a refcounting problem somewhere,
> > either introduced, or exposed by the patch.
> 
>  well, if i remove the code from GDOMBinding.cpp which is designed to
>  help debug refcounting problems, we can't FIND OUT, CAN WE!!!

>  and yes, at the request of the reviewers, i have been forced to remove it,
>  because it had "#ifdef 0" around it.
> 
>  explanations in the code from which it was cut-and-paste - from JSBindings -
>  state that the code is [even #ifdef'd out there] necessary to check for
>  refcounting errors.

The equivalent code in JSDOMBindings.cpp is not wrapped in #if 0.  It is code
that is compiled in and actively used.  For this code to be useful in the GDOM
bindings it too needs to be actively used.  Having it #if'd out does not
accomplish that.  It should either be left in and actively used, or removed and
replaced with a FIXME about adding such functionality in the future.

>  i am reasonably confident that, having used RefPtr in the right places,
>  and having done ref increasing in the constructors, and ref decreasing
>  in the destructors, i am reasonably confident that there are not any
>  refcount errors.

I've not looked at code generated by the bindings script, but from inspection
of the script itself and the hand-written code I can see several errors.

 678         push(@cBody, "    m_impl.get()->deref();\n");

Explicitly calling deref() on a reference held in a RefPtr is a sure sign that
ref-counting is being handled incorrectly.  A RefPtr will automatically call
deref() when it is destroyed, so by calling deref explicitly you're
decrementing the ref-count on this object twice.

I don't think the generated bindings code should be calling ref() or deref()
explicitly at all.  Managing the lifetimes of the wrapped objects via a RefPtr
is almost certainly sufficient.

The manual calls to ref() and deref() in GDOMBinding.cpp also hint at
ref-counting being handled incorrectly, and perhaps indicate why you believed
it was necessary to deref() from within the generated code.

 506     text->document()->ref(); 

If a manual ref() were required here, which it's not, ref'ing the document is
definitely not correct here.


There is some documentation at <http://webkit.org/coding/RefPtr.html> on how to
use RefPtr and PassRefPtr to efficiently handle our reference counted objects.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.