GNOME Bugzilla – Bug 756090
Variant.new_from_data() does not sink or ref the newly-created GVariant
Last modified: 2018-05-22 15:28:03 UTC
I am looking into a crash in gnome-contacts, which looks like a double-unref of a GVariant: https://bugzilla.redhat.com/show_bug.cgi?id=1244256 The Vala code at fault is the avatar_icon_data property getter: https://git.gnome.org/browse/gnome-contacts/tree/src/contacts-contact.vala#n158 Specifically, Variant.new_from_data() seems to be at fault, when looking at the generated C code: _tmp12_ = G_VARIANT_TYPE_BYTESTRING; _tmp13_ = contacts_contact_get_small_avatar (self); _tmp14_ = _tmp13_; _tmp16_ = gdk_pixbuf_get_pixels_with_length (_tmp14_, &_tmp15_); _tmp17_ = contacts_contact_get_small_avatar (self); _tmp18_ = _tmp17_; _tmp19_ = _g_object_ref0 (_tmp18_); _tmp20_ = g_variant_new_from_data (_tmp12_, _tmp16_, _tmp15_, TRUE, g_object_unref, _tmp19_); pixel_data = _tmp20_; _tmp21_ = contacts_contact_get_small_avatar (self); _tmp22_ = _tmp21_; _tmp23_ = gdk_pixbuf_get_width (_tmp22_); _tmp24_ = contacts_contact_get_small_avatar (self); _tmp25_ = _tmp24_; _tmp26_ = gdk_pixbuf_get_height (_tmp25_); _tmp27_ = contacts_contact_get_small_avatar (self); _tmp28_ = _tmp27_; _tmp29_ = gdk_pixbuf_get_rowstride (_tmp28_); _tmp30_ = contacts_contact_get_small_avatar (self); _tmp31_ = _tmp30_; _tmp32_ = gdk_pixbuf_get_has_alpha (_tmp31_); _tmp33_ = contacts_contact_get_small_avatar (self); _tmp34_ = _tmp33_; _tmp35_ = gdk_pixbuf_get_bits_per_sample (_tmp34_); _tmp36_ = contacts_contact_get_small_avatar (self); _tmp37_ = _tmp36_; _tmp38_ = gdk_pixbuf_get_n_channels (_tmp37_); _tmp39_ = pixel_data; _tmp40_ = g_variant_new ("(iiibii@ay)", _tmp23_, _tmp26_, _tmp29_, _tmp32_, _tmp35_, _tmp38_, _tmp39_, NULL); g_variant_ref_sink (_tmp40_); _g_variant_unref0 (self->priv->_avatar_icon_data); self->priv->_avatar_icon_data = _tmp40_; _g_variant_unref0 (pixel_data); In other words, the pixel_data GVariant is not reffed, nor does it have its floating ref sunk, so the last _g_variant_unref0() causes it to be freed.
This needs some work. If we turn the method into a constructor, then <T> will not be supported. If we stay with the static method, we have to fix the behavior. Probably it's easier to stay with the static method, also for compatibility.
Seems like the code now has changed. Do you have a test case? This code correctly ref_sink the variant for me: void main() { uchar[] data = {0}; var v = Variant.new_from_data<Object?> (VariantType.BYTESTRING, data, true, null); } Generated: _tmp1_ = G_VARIANT_TYPE_BYTESTRING; _tmp2_ = g_variant_new_from_data (_tmp1_, data, data_length1, TRUE, g_object_unref, NULL); g_variant_ref_sink (_tmp2_); v = _tmp2_; _g_variant_unref0 (v);
(In reply to Luca Bruno from comment #2) > Seems like the code now has changed. Do you have a test case? I worked around the Vala bug by using g_icon_serialize(), so no. > This code correctly ref_sink the variant for me: You can see the old code, before the workaround https://git.gnome.org/browse/gnome-contacts/tree/src/contacts-contact.vala?id=1333a956082a4d14b0de2880fe595e10e20496c7#n158
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vala/issues/514.