Potential Memory Safety Issue: Possible deallocator mismatch in `ConstraintLayout::add_constraints_from_description`

[zeroy0410]

Hi there,

While reading the source for ConstraintLayout::add_constraints_from_description, I noticed a detail that might be a potential memory safety issue and wanted to bring it to your attention.

Potential Issue

In this function, a GHashTable is created using g_hash_table_new_full, and g_free is set as the destroy notify function for its values.

let hash_table = glib::ffi::g_hash_table_new_full(
    // ...
    Some(glib::ffi::g_free), // key_destroy
    Some(glib::ffi::g_free), // value_destroy
);

In the following loop, the table is populated by calling widget.to_glib_full(). Based on the typical implementation in glib bindings, this method usually calls g_object_ref, incrementing the widget's reference count.

If that's the case here, there appears to be a mismatch: ownership is acquired via reference counting, but the function to release it is set to g_free. This could lead to an attempt to call g_free on a GObject pointer when the hash table is destroyed, which would likely cause heap corruption or a crash.

Suggested Fix

If my understanding of the code is correct, a possible fix would be to change the value_destroy_func to g_object_unref.

let hash_table = glib::ffi::g_hash_table_new_full(
    Some(glib::ffi::g_str_hash),
    Some(glib::ffi::g_str_equal),
    Some(glib::ffi::g_free),            // For string keys
    Some(gobject_ffi::g_object_unref), // For GObject values
);

Of course, I may not have the full context for this part of the code, so I'm happy to be corrected.

Thanks for your great work on this project!

[sdroege]

Yes it's using the wrong free function, but it's not a memory safety issue because it's also leaking the hash table 🤣 Fixed both in #2120

Thanks for reporting!