hugopl / gi-crystal

Tool to generate Crystal bindings for gobject-based libraries (i.e. GTK)
BSD 3-Clause "New" or "Revised" License
45 stars 3 forks source link

Fixes random crashes caused by bad wrapper instance cache. #133

Closed hugopl closed 11 months ago

hugopl commented 11 months ago

When you create a GObject, GI-Crystal adds a qdata to it with a pointer to the Crystal object, so if this object goes into C world and appear later in e.g. a signal callback, the Crystal object can be restored and all Crystal code there, including memory for the isntance variables will work as expected.

To avoid this object to be collected by GC is another subject.

This behavior is needed by user types, because we must be able to access the instance variables, etc. But for wrappers this isn't needed since it only holds a pointer to the C object, but GICrystal was doing this anyway to save some memory allocations, as we know:

Early optimization is the root of all evil

The problem is:

If you a signal with a GObject parameter that is really a Gtk::ListView the signal code will do GObject::Object.new(pointer, :none).

This call will create the Crystal instance and save it in the qdata, so later when you do a cast like:

Gtk::ListView.cast it seems to work, but it is returning the Crystal object for the GObject::Object memory layout, due to unsafe code the compiler will believe us and think it's a Gtk::ListView.

When you try to use some specific Gtk::ListView method a crash can happen depending on how the compiler decided to do such method call, since the type id is for a different type it will just crash, I'm not sure if Crystal uses a vtable for that, if so this also explains the error.

Solution

Remove the cache for non-user types fixes the problem. But to make wrapper objects more Crystal-like the self.new was modified to identify the GType and call the right wrapper .new method for it.

This means that is possible now to use Crystal casts for wrapper objects!

This opens the possibility to avoid the Abstract classes created for each GObject interface, since we could simple do:

GObject::Object.new(ptr, :none).as(Interface)

And I tried, but crashed the compiler 😅.