Re: Problems with un-owned objects passed to closures in pygobject (gtk_cell_renderer_text_start_editing)

[Owen Taylor <otaylor redhat com>]

Hi Simon,

I didn't see this thread earlier. I wanted to chime in to strongly
support the view that:

 * Floating references are C-convenience only

 * Languages can and should sink floating references when they
   first touch them.

 * Any interface or code in libraries that create problems with
   this are buggy.

This has been the view from day 1, and I think you'll create
considerably bigger problems trying to futz around with it and treat
floating references specially in a binding than you have now.

On Tue, 2013-02-05 at 05:33 -0800, Simon Feltman wrote:
> For completeness, the two major problems are as follows

> https://bugzilla.gnome.org/show_bug.cgi?id=687522
> This is a vfunc implementation which the gtk internals are basically
> expecting a floating ref from. Using the standard scheme just listed,
> we sink and own the created MenuToolButton. The held widget is then
> finalized at the end of the vfunc, returning an invalid object back to
> the caller. If we add an extra ref we get a leak because the method is
> marked as transfer-none. Example:
> 
> 
> class ToolMenuAction(Gtk.Action):
>     def do_create_tool_item(self):
> 
>         return Gtk.MenuToolButton()

This is basically broken API at the GTK+ level :-( ... a virtual
function can't return (transfer none) unless it's a getter for an
existing field. It is expected that language bindings will have no way
to create a floating object, so a virtual function cannot expect to be
returned a floating object.

The only thing I can see at the GTK+ level would be to add a
make_tool_item replacement vfunc and use that instead if non-null.
There's a workaround at the application level, which is something like:

     def do_create_tool_item(self):
         button = Gtk.MenuToolButton()
         self.buttons.append(button)
         button.connect('destroy', self.remove_from_buttons)
 
         return button

we can document this bug in the gtk-doc and suggest the workaround
there. But I'd strongly suggest *not* doing wholesale changes to the
Python memory management based on this bug.

> https://bugzilla.gnome.org/show_bug.cgi?id=661359
> This is a very simple case of a widget as a parameter being marshaled
> as an in arg to a callback. But because the gtk internals have not yet
> sunk the floating ref for the "editable" parameter, PyGObject will do
> so. By the time the callback is finished, the editable will be
> finalized leaving gtk with a bad object. It should really just be
> adding a safety ref during the lifetime of the wrapper and not mess
> with the floating flag.
> 
> 
> def on_view_label_cell_editing_started(renderer, editable, path):
> 
>     print path
> renderer = Gtk.CellRendererText()
> renderer.connect('editing-started',
> on_view_label_cell_editing_started)

This one is is simple. GTK+ needs to sink the arg before calling the
function. There should be no compatibility problems. The pygi patch on
the bug appears simply wrong and probably is creating the leak you
noticed.

- Owen

(I apologize for any duplication of earlier discussion on the thread)