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

[Simon Feltman <s feltman gmail com>]

Hi Everyone,

I'm trying to figure out a complete solution for proper reference management of gobjects passed into python vfuncs and signal closures as "in" arguments. Currently pygobject will add a ref to gobject arguments marked as transfer-none during marshaling and additionally sink/ref the gobject when the python object wrapper is created around it. The initial added ref during marshaling is then leaked upon the closure finishing in most cases. 

The fix could simply be to not add the initial ref during marshaling (or we need to make sure it is released when the closure finishes). The problem is this behavior is relied upon when a floating ref is passed and is the explicit fix described in the following ticket (which un-fortunately causes a leak in the non-floating case):

https://bugzilla.gnome.org/show_bug.cgi?id=661359

The specific problem described in bug 661359 occurs when a python closure is connected to a GtkCellRendererTexts "editing-started" signal. During the start of editing, a GtkEntry is created and passed almost immediately to signal emission before any ownership takes place:

http://git.gnome.org/browse/gtk+/tree/gtk/gtkcellrenderer.c#n864

This results in the following ref counting behavior of the GtkEntry as it is used by gtk_cell_area_activate_cell and gtk_cell_renderer_start_editing:

1 (floating) - gtk_cell_renderer_text_start_editing creates the GtkEntry

2 (floating) - g_signal_emit argument marshaling adds an additional ref during signal emission

3 (floating) - python closure marshaling adds an additional ref described above

3 (owned) - A PyGObject wraps the GtkEntry and sinks the floating ref

* python callback is called and passed the wrapper

2 (owned) - The PyGObject wrapper is destroyed after the python callback finishes additionally freeing a gobject ref

1 (owned) - g_signal_emit argument marshaling completes and frees the ref it added

2 (owned) - gtk_cell_area_activate_cell sets the CellAreas edit widget which adds a ref

1 (owned) - cell editing finishes and removes the widget from the CellArea

Given the above it seems cell editing will leak a new editor each time editing occurs even if no closures are connected to the "editing-started" signal (python or otherwise). The pygobject specific problems I mention could be solved by tracking incoming floating refs and re-floating them upon closure exit if the python ref is not stored anywhere (or by attempting to just keep it floating). However, the root cause of all this could simply be that cell editing needs better reference management of the editors being created. For instance, gtk_cell_renderer_text_start_editing creates the GtkEntry and stores it as an attribute in GtkCellRendererText.priv.entry without sinking the ref:

http://git.gnome.org/browse/gtk+/tree/gtk/gtkcellrenderertext.c#n2049

I think this should be changed to immediately sink the ref and unref it during gtk_cell_renderer_text_editing_done. This would solve the python problems and fix the reference leak for the editor (and bug 661359 can be backed out). Additionally I think gtk_cell_area_set_edit_widget should be sinking the incoming editable widget instead of simply adding a ref:

http://git.gnome.org/browse/gtk+/tree/gtk/gtkcellarea.c#n3309

As a general practice shouldn't refs be sunk when they are being assigned to a structured attribute especially when dealing with GInitiallyUnowned based objects?

Insight into any of this is greatly appreciated.

-Simon