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



Owen,

Thank you, this will definitely make things easier. Comments below:

 * Floating references are C-convenience only
 
It might be a convenience for writing code, not necessarily reading or understanding it. Something more reasonable would be explicitly named convenience functions which steal a ref instead of adding one. But that may only be an alternate reality at this point.

 * 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.

I will submit bug reports and patches where applicable.

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.

Agreed.

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.

Docs will help, we can also detect the bad situation and assert giving some explanation to minimize support load and keep future maintainers from trying to fix it again. Would something like the following be safe in regards to testing the GObject ref count?

assert !(transfer == nothing && pyobject->ref_count == 1 && gobject->ref_count ==1)

This means the Python wrapper is only held in the out args tuple and will be free'd when the closure is finalized, deleting the gobject along with it and giving back a bad object. We could also add an additional ref to the gobject and throw up a nasty warning.

Unfortunately there are a few more of these:
Gtk.Action.create_menu
Gtk.Action.create_menu_item
Gtk.Action.create_tool_item
Gtk.PrintOperation.create_custom_widget
Gtk.PrintOperation.create-custom-widget (signal)
Gladeui.EditorProperty.create_input
Gladeui.BaseEditor.build_child?
Gladeui.BaseEditor.build-child (signal)

> 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.

Yes, definitely a leak. However, fixing it will break people again. So the gtk bug needs to be fixed first. There is also no telling how many of these there are without looking through all vfuncs/signals which take a widget as an input arg with transfer=none.

Thanks,
-Simon




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]