GNOME Bugzilla – Bug 746893
gtk_list_box_bind_model is not introspectable
Last modified: 2015-06-02 15:20:58 UTC
Please quit implementing features that are only usable in C.
Please, instead of snappy remarks, could you please explain why is not bindable? Is it because of missing type information on the GtkListBoxCreateWidgetFunc closure?
In the gir, I see: <method name="bind_model" c:identifier="gtk_list_box_bind_model" version="3.16" introspectable="0"> It might be interesting to figure out if we can add 'bindability' tests to our testsuite.
Likely because GtkListBoxCreateWidgetFunc is also marked as not introspectable: <callback name="ListBoxCreateWidgetFunc" c:type="GtkListBoxCreateWidgetFunc" version="3.16" introspectable="0"> <doc xml:space="preserve">Called for list boxes that are bound to a #GListModel with gtk_list_box_bind_model() for each item that gets added to the model.</doc> <return-value> <doc xml:space="preserve">a #GtkWidget that represents @item</doc> <type name="Widget" c:type="GtkWidget*"/> </return-value> <parameters> <parameter name="item" transfer-ownership="none"> <doc xml:space="preserve">the item from the model for which to create a widget for</doc> <type name="gpointer" c:type="gpointer"/> </parameter> <parameter name="user_data" transfer-ownership="none" closure="1"> <doc xml:space="preserve">user data</doc> <type name="gpointer" c:type="gpointer"/> </parameter> </parameters> </callback>
Indeed, with this: diff --git a/gtk/gtklistbox.h b/gtk/gtklistbox.h index 4ff8383..a648ac8 100644 --- a/gtk/gtklistbox.h +++ b/gtk/gtklistbox.h @@ -171,13 +171,13 @@ typedef void (*GtkListBoxUpdateHeaderFunc) (GtkListBoxRow *row, /** * GtkListBoxCreateWidgetFunc: - * @item: the item from the model for which to create a widget for + * @item: (type GObject): the item from the model for which to create a widget for * @user_data: (closure): user data * * Called for list boxes that are bound to a #GListModel with * gtk_list_box_bind_model() for each item that gets added to the model. * - * Returns: a #GtkWidget that represents @item + * Returns: (transfer full): a #GtkWidget that represents @item * * Since: 3.16 */ Both bind_model() and GtkListBoxCreateWidgetFunc are introspectable. The transfer rules of the delegate function are fuzzy, though; the GtkListBox will just take the pointer, but it does not state whether or not the ownership of the returned pointer is meant to be transferred fully.
So, it's apparently clear that the API (along with GListModel) is not really meant to be bindable through introspection — and minimally bindable only by open coding the callable trampoline. I suspect this is going to be a WONTFIX, or it's going to be made dependant of an eventual bug for making introspection work with collections.
I'm not sure if this matches the implementation, but transfer full makes the most sense to me. The expectation is that the widget returned by the create func will be immediately added as a child of the ListBox (or added to a ListBoxRow and added to the ListBox), so it would be taking ownership of the reference initially held in the create func.
Created attachment 300471 [details] [review] GtkListBox: fix model binding refcount issue As it is, GtkListBox model binding will work nicely as long as your create_widget_func returns a floating reference on the newly-created widget. If you try to return a full reference (as any higher-level language would do) then you will leak that reference. Fix that up by converting any floating references into full references and then unconditionally releasing the full reference after adding to the box.
Review of attachment 300471 [details] [review]: Looks good. Thanks for the big comment on top.
(In reply to Ryan Lortie (desrt) from comment #7) > Created attachment 300471 [details] [review] [review] This is technically an API break since we will change the behaviour in the case that the user gives us a full ref. Previously, we would leak that ref, but now we will release it ("properly"). The only problem is if the user was somehow arranging to have that ref release later themselves. Let's backport this to the stable branch before anyone notices.... it's only been a released stable API for a few days.
Comment on attachment 300471 [details] [review] GtkListBox: fix model binding refcount issue Attachment 300471 [details] pushed as 6e03e7e - GtkListBox: fix model binding refcount issue
...and pushed to gtk-3-16.
Hold up, though. I appreciate the refcount change, but GtkListBoxCreateWidgetFunc is still marked as introspectable=0 without something like the change in comment 4
I'm sorry -- git-bz marked the bug as closed by accident. I didn't mean to imply that it was fixed. fwiw, I think the approach in comment 4 is approximately correct.
Review of attachment 300471 [details] [review]: ::: gtk/gtklistbox.c @@ +3618,3 @@ + * Finally, we'll release this full reference below, leaving only + * the one held by the box. + */ Actual use from gjs disagrees with this assessment. For example, see the WIP patch in bug 726450 Without this, the GtkListBoxRows are definitely not being leaked because I can see gtk_list_box_row_finalize being invoked by the GC. On the other hand, this patch leads to a crash with "Finalizing proxy for an already freed object of type: GtkListBoxRow" when the GC runs.
Reopened.
(In reply to Debarshi Ray from comment #14) > Actual use from gjs disagrees with this assessment. For example, see the WIP > patch in bug 726450 > > Without this, the GtkListBoxRows are definitely not being leaked because I > can see gtk_list_box_row_finalize being invoked by the GC. > > On the other hand, this patch leads to a crash with "Finalizing proxy for an > already freed object of type: GtkListBoxRow" when the GC runs. After talking to desrt and Jasper in #gtk+ on GIMPNet, I realized that this is the right thing to do. The fault lies with gjs (bug 750286) in this case. Closing.