Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GICrystal::ObjectCollectedError when trying to bind to a property from a UiTemplate #69

Open
thatcher-gaming opened this issue Jun 11, 2024 · 5 comments · May be fixed by hugopl/gi-crystal#153
Labels
bug Something isn't working

Comments

@thatcher-gaming
Copy link

Using a property binding from within a .ui file will result in a GICrystal::ObjectCollectedError being raised.

Here's an example:

require "gtk4"

APP_ID = "com.example.test"

@[Gtk::UiTemplate(file: "src/test.ui")]
class TestWindow < Gtk::ApplicationWindow
  include Gtk::WidgetTemplate

  @[GObject::Property]
  property test = "hello"

  def initialize()
    super()
  end
end

app = Gtk::Application.new(APP_ID, Gio::ApplicationFlags::None)

app.activate_signal.connect do
  window = TestWindow.new
  window.application = app

  window.present
end

exit(app.run(ARGV))

test.ui:

<?xml version="1.0" encoding="UTF-8"?>
<interface>
  <requires lib="gtk" version="4.0"/>
  <template class="TestWindow" parent="GtkApplicationWindow">
    <property name="title" bind-source="TestWindow" bind-property="test" bind-flags="sync-create"/>
  </template>
</interface>

Output:

Unhandled exception:  (GICrystal::ObjectCollectedError)
  from src/test.cr:6:1 in '_vfunc_unsafe_get_property'
  from src/test.cr:6:1 in '->'
  from /lib64/libgobject-2.0.so.0 in 'g_object_get_property'
  from /lib64/libgobject-2.0.so.0 in '??'
  from /lib64/libgobject-2.0.so.0 in 'g_object_bind_property_full'
  from /lib64/libgobject-2.0.so.0 in 'g_object_bind_property'
  from /lib64/libgtk-4.so.1 in '??'
  from /lib64/libgtk-4.so.1 in 'gtk_builder_extend_with_template'
  from /lib64/libgtk-4.so.1 in 'gtk_widget_init_template'
  from src/test.cr:7:3 in '_instance_init'
  from src/test.cr:6:1 in '->'
  from /lib64/libgobject-2.0.so.0 in 'g_type_create_instance'
  from /lib64/libgobject-2.0.so.0 in '??'
  from /lib64/libgobject-2.0.so.0 in 'g_object_newv'
  from lib/gi-crystal/src/bindings/g_object/object.cr:607:18 in 'initialize'
  from lib/gi-crystal/src/auto/g_object-2.0/initially_unowned.cr:17:7 in 'initialize'
  from lib/gi-crystal/src/auto/gtk-4.0/widget.cr:26:7 in 'initialize'
  from lib/gi-crystal/src/auto/gtk-4.0/window.cr:35:7 in 'initialize'
  from lib/gi-crystal/src/auto/gtk-4.0/application_window.cr:41:7 in 'initialize'
  from src/test.cr:13:5 in 'initialize'
  from src/test.cr:12:3 in 'new'
  from src/test.cr:20:12 in '->'
  from lib/gi-crystal/src/auto/gio-2.0/application.cr:770:44 in '->'
  from /lib64/libgobject-2.0.so.0 in 'g_closure_invoke'
  from /lib64/libgobject-2.0.so.0 in '??'
  from /lib64/libgobject-2.0.so.0 in '??'
  from /lib64/libgobject-2.0.so.0 in 'g_signal_emit_valist'
  from /lib64/libgobject-2.0.so.0 in 'g_signal_emit'
  from /lib64/libgio-2.0.so.0 in '??'
  from /lib64/libgio-2.0.so.0 in 'g_application_run'
  from lib/gi-crystal/src/auto/gio-2.0/application.cr:544:7 in 'run'
  from src/test.cr:26:6 in '__crystal_main'
  from /usr/share/crystal/src/crystal/main.cr:129:5 in 'main_user_code'
  from /usr/share/crystal/src/crystal/main.cr:115:7 in 'main'
  from /usr/share/crystal/src/crystal/main.cr:141:3 in 'main'
  from /lib64/libc.so.6 in '??'
  from /lib64/libc.so.6 in '__libc_start_main'
  from /home/leah/.cache/crystal/crystal-run-test.tmp in '_start'
  from ???
@hugopl hugopl added the bug Something isn't working label Jun 12, 2024
@hugopl
Copy link
Owner

hugopl commented Jun 12, 2024

The problem seems to be the order of creation, so when the C code calls the Crystal property the Crystal object isn't fully initialized yet.

@hugopl
Copy link
Owner

hugopl commented Jun 12, 2024

Problem:

GICrystal stores a qdata in each GObject to inform the pointer of the Crystal object for that GObject, so when C code asks for a GObject property defined in Crystal it knows how to get it.

This qdata is set at lib/gi-crystal/src/bindings/g_object/object.cr:609

    def initialize
      @pointer = LibGObject.g_object_newv(self.class.g_type, 0, Pointer(LibGObject::Parameter).null)
      LibGObject.g_object_ref_sink(self) if LibGObject.g_object_is_floating(self) == 1
      LibGObject.g_object_set_qdata(self, GICrystal::INSTANCE_QDATA_KEY, Pointer(Void).new(object_id))
      self._after_init
    end

But... for templates, the GObject property is read before we reach the line 609, it's read when the GObject is created on LibGObject.g_object_newv call. So Crystal code is asked for a property value but it has no means to know the Crystal object instance where it can get this value from.

Possible solution that I did pop up from my head but need more study:

  • Save the Crystal instance pointer in a read-only, construct only property, so it's possible to set it at GObject construction. (Cons: probably it's slower than use qdata; Will be harder to make Crystal objects instantiated from C in the future)
  • Have a thread local variable to set the qdata value while it's not yet set on GObject (Cons: hacky and "ugly")

hugopl added a commit to hugopl/gi-crystal that referenced this issue Jun 24, 2024
This is a breaking change, since it now requires all GObject subclasses
to have a constructor without arguments.

What changed:

There are 2 thread local variables to inform if the object is being created
in C land or Crystal land, we store the objects instance pointers there.

When the object is created in C land, the Crystal instance is created on
GObject instance_init method.

When the object is created in Crystal land, the GObject instance_init method
creates no Crystal instance.

This also fixes the use case of tryign to use a Crystal defined GObject property
before the Wrapper gets fully initialized.

Fixes hugopl/gtk4.cr#69
hugopl added a commit to hugopl/gi-crystal that referenced this issue Jun 24, 2024
This is a breaking change, since it now requires all GObject subclasses
to have a constructor without arguments.

What changed:

There are 2 thread local variables to inform if the object is being created
in C land or Crystal land, we store the objects instance pointers there.

When the object is created in C land, the Crystal instance is created on
GObject instance_init method.

When the object is created in Crystal land, the GObject instance_init method
creates no Crystal instance.

This also fixes the use case of tryign to use a Crystal defined GObject property
before the Wrapper gets fully initialized.

Fixes hugopl/gtk4.cr#69
hugopl added a commit to hugopl/gi-crystal that referenced this issue Jun 24, 2024
This is a breaking change, since it now requires all GObject subclasses
to have a constructor without arguments.

What changed:

There are 2 thread local variables to inform if the object is being created
in C land or Crystal land, we store the objects instance pointers there.

When the object is created in C land, the Crystal instance is created on
GObject instance_init method.

When the object is created in Crystal land, the GObject instance_init method
creates no Crystal instance.

This also fixes the use case of tryign to use a Crystal defined GObject property
before the Wrapper gets fully initialized.

Fixes hugopl/gtk4.cr#69
hugopl added a commit to hugopl/gi-crystal that referenced this issue Jun 24, 2024
This is a breaking change, since it now requires all GObject subclasses
to have a constructor without arguments.

What changed:

There are 2 thread local variables to inform if the object is being created
in C land or Crystal land, we store the objects instance pointers there.

When the object is created in C land, the Crystal instance is created on
GObject instance_init method.

When the object is created in Crystal land, the GObject instance_init method
creates no Crystal instance.

This also fixes the use case of tryign to use a Crystal defined GObject property
before the Wrapper gets fully initialized.

Fixes hugopl/gtk4.cr#69
hugopl added a commit to hugopl/gi-crystal that referenced this issue Jun 24, 2024
This is a breaking change, since it now requires all GObject subclasses
to have a constructor without arguments.

What changed:

There are 2 thread local variables to inform if the object is being created
in C land or Crystal land, we store the objects instance pointers there.

When the object is created in C land, the Crystal instance is created on
GObject instance_init method.

When the object is created in Crystal land, the GObject instance_init method
creates no Crystal instance.

This also fixes the use case of tryign to use a Crystal defined GObject property
before the Wrapper gets fully initialized.

Fixes hugopl/gtk4.cr#69
hugopl added a commit to hugopl/gi-crystal that referenced this issue Jun 24, 2024
This is a breaking change, since it now requires all GObject subclasses
to have a constructor without arguments.

What changed:

There are 2 thread local variables to inform if the object is being created
in C land or Crystal land, we store the objects instance pointers there.

When the object is created in C land, the Crystal instance is created on
GObject instance_init method.

When the object is created in Crystal land, the GObject instance_init method
creates no Crystal instance.

This also fixes the use case of tryign to use a Crystal defined GObject property
before the Wrapper gets fully initialized.

Fixes hugopl/gtk4.cr#69
hugopl added a commit to hugopl/gi-crystal that referenced this issue Jun 24, 2024
This is a breaking change, since it now requires all GObject subclasses
to have a constructor without arguments.

What changed:

There are 2 thread local variables to inform if the object is being created
in C land or Crystal land, we store the objects instance pointers there.

When the object is created in C land, the Crystal instance is created on
GObject instance_init method.

When the object is created in Crystal land, the GObject instance_init method
creates no Crystal instance.

This also fixes the use case of tryign to use a Crystal defined GObject property
before the Wrapper gets fully initialized.

Fixes hugopl/gtk4.cr#69
@hugopl
Copy link
Owner

hugopl commented Jun 24, 2024

Hi, if you could test hugopl/gi-crystal#153 to double check if the issue was fixed I would appreciate :-)

To use this version of gi-crystal you need to use a shards.override.yml file.

@thatcher-gaming
Copy link
Author

@hugopl That does indeed seem to have done the trick. Many thanks for the fix!

@hugopl
Copy link
Owner

hugopl commented Jun 25, 2024

Thanks for testing it! (and also for reporting the issue!)

Good thing about this fix is that now will also be possible to create Crystal deffined widgets from GtkBuild files :-), I didn't tested yet... but in theory it works.

I'll compile Tijolo with this patch and use it some days just to check if it's not doing anything bad before merge it.

This patch and the future patch to have signal connections that doesn't leak objects will grant a good leap in quality for these bindings.

hugopl added a commit to hugopl/gi-crystal that referenced this issue Jun 28, 2024
This is a breaking change, since it now requires all GObject subclasses
to have a constructor without arguments.

What changed:

There are 2 thread local variables to inform if the object is being created
in C land or Crystal land, we store the objects instance pointers there.

When the object is created in C land, the Crystal instance is created on
GObject instance_init method.

When the object is created in Crystal land, the GObject instance_init method
creates no Crystal instance.

This also fixes the use case of tryign to use a Crystal defined GObject property
before the Wrapper gets fully initialized.

Fixes hugopl/gtk4.cr#69
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants