jhass / crystal-gobject

gobject-introspection for Crystal
BSD 3-Clause "New" or "Revised" License
127 stars 13 forks source link

GC seems to unref objects twice #88

Open hugopl opened 3 years ago

hugopl commented 3 years ago

Hi,

I'm having some issues with gobject unref's generating some GTK warnings, the reduced code can be found bellow.

It just create a instance of Foo and let the GC destroy it, on first GC.collect everything is fine:

However on the second call to GC.collect it tries to destroy something that I have no idea what, but looking at the warnings it seems like something already destroyed.

Here's the code:

require "gobject/gtk/autorun"

class Foo
  def initialize
    @window = Gtk::Window.new(title: "Hello World!")
    @window.connect("destroy") do
      puts "destroyed!"
      GLib.idle_add do
        puts "real goodbye!"
        Gtk.main_quit
        false
      end
    end
    @window.show
  end

  def hold_a_reference_in_a_closure(window)
    puts "I got focus"
  end

  def finalize
    LibC.printf("finalize!\n")
  end
end

# need to be on a block, otherwise for some reason GC wont collect
1.times do
  puts "Foo.new"
  Foo.new
end
puts "collect 1"
GC.collect
puts "collect 2"
GC.collect

and the output:

Foo.new
collect 1
finalize!
collect 2
destroyed!

(crash:28464): GLib-GObject-CRITICAL **: 16:48:04.048: g_object_set_qdata: assertion 'G_IS_OBJECT (object)' failed

(crash:28464): GLib-GObject-WARNING **: 16:48:04.048: instance with invalid (NULL) class pointer

(crash:28464): GLib-GObject-CRITICAL **: 16:48:04.048: g_signal_handlers_destroy: assertion 'G_TYPE_CHECK_INSTANCE (instance)' failed

(crash:28464): GLib-GObject-WARNING **: 16:48:04.048: instance with invalid (NULL) class pointer

(crash:28464): GLib-GObject-CRITICAL **: 16:48:04.048: g_signal_handlers_destroy: assertion 'G_TYPE_CHECK_INSTANCE (instance)' failed

(crash:28464): GLib-GObject-CRITICAL **: 16:48:04.048: g_object_unref: assertion 'old_ref > 0' failed
real goodbye!

I generated a core file from this warnings by running the application with G_DEBUG=fatal-warnings env var, and here's the stack trace of the first warning.

#0  0x00007f7d5908758b in g_logv () at /usr/lib/libglib-2.0.so.0
#1  0x00007f7d59087860 in g_log () at /usr/lib/libglib-2.0.so.0
#2  0x00007f7d594f47b6 in  () at /usr/lib/libgtk-3.so.0
#3  0x00007f7d594ff9bd in  () at /usr/lib/libgtk-3.so.0
#4  0x00007f7d5917f591 in g_object_unref () at /usr/lib/libgobject-2.0.so.0
#5  0x000055f64665e5dd in finalize () at /home/hugo/src/gtkcrash/lib/gobject/src/gtk/gtk.cr:4
#6  0x000055f6465f134f in -> () at /usr/lib/crystal/gc/boehm.cr:164
#7  0x00007f7d58e1a6df in GC_invoke_finalizers () at /usr/lib/libgc.so.1
#8  0x00007f7d58e1a8f1 in  () at /usr/lib/libgc.so.1
#9  0x00007f7d58e201be in  () at /usr/lib/libgc.so.1
#10 0x00007f7d58e2029d in GC_gcollect () at /usr/lib/libgc.so.1
#11 0x000055f64660db96 in collect () at /usr/lib/crystal/gc/boehm.cr:135
#12 0x000055f6465d8b7c in __crystal_main () at /home/hugo/src/gtkcrash/src/crash.cr:38
#13 0x000055f646662bf6 in main_user_code () at /usr/lib/crystal/crystal/main.cr:110
#14 0x000055f6465f0faa in main () at /home/hugo/src/gtkcrash/lib/gobject/src/gtk/autorun.cr:7

#5 0x000055f64665e5dd in finalize () at /home/hugo/src/gtkcrash/lib/gobject/src/gtk/gtk.cr:4 is the require_gobject macro, nto of great help, however it's probably from Gtk::Window finalizer... so it seems to be destroying the object twice.

Environment: ArchLinux, crystal 1.0.0, LLVM 10.0.01, glib 2.68, gtk 3.24.29.

jhass commented 3 years ago

We're currently completely ignoring the ownership tags, so it's probably unrefing some container which didn't ref the pointer but should have. But just a guess, no debugging done so far.

hugopl commented 3 years ago

Good news... the bug is much more simple... I translated the code snippet to C and reproduced the very same double unref, so I come to the conclusion that I should stop and read the docs :rofl:

#include <gtk/gtk.h>

int main(int argc, char *argv[]) {
  gtk_init(&argc, &argv);
  GtkWidget* wnd = gtk_window_new(0);

  gtk_widget_show(GTK_WIDGET(wnd));

  g_object_unref(G_OBJECT(wnd));
  gtk_main();
}

The documentation is clear: To delete a GtkWindow, call gtk_widget_destroy(), the same for any GtkWidget subclass.

In the example above if I replace the g_object_unref call by a gtk_widget_destroy it works as expected. One thing that I didn't check yet is if there is some GIR annotation informing that or if the binding generator must hardcode this rule.

hugopl commented 3 years ago

GIR has the info, https://developer.gnome.org/gi/stable/gi-GIObjectInfo.html#g-object-info-get-unref-function

I saw you call unref for the the object is a ObjectInfo or InterfaceInfo at src/g_i_repository/wrapper_generator.cr:33,

      if self.is_a?(ObjectInfo) || self.is_a?(InterfaceInfo)
        builder.def_method("finalize") do
          line call("object_unref", call("as", "LibGObject::Object*", receiver: "@pointer"), receiver: "LibGObject")
        end
      end

My doubt now is... if we do this:

def build_wnd
  wnd = Gtk::Window.new(...)
  Gtk::Buildable.cast(wnd)
end

def build_interface
  interface = build_wnd
  5.times { GC.collect } # Collect the `wnd` created on build_wnd and calls widget_destroy, that just unref because interface holds a ref.
end

build_interface
5.times { GC.collect } # collect interface, but calls `unref`, what happen since the object is really a GtkWindow?

My doubt is mainly because I know very little how GTK interfaces behave and its limitations, so I'm thinking with C++ism in my head, but by my logic it would call unref on GtkWindow instead of destroy and the double unref would happen.

To late here, other day I can test this interface use case in C to see what happen or.... RTFM :grin:

hugopl commented 2 years ago

I got this very same problem on my gtk4 shard. The fix was simple, if the GObject inherits from GInitiallyUnowned it must call g_object_ref_sink in the constructor to sink the floating reference[1].

[1] https://docs.gtk.org/gobject/floating-refs.html

phil294 commented 1 year ago

not sure this is the exact same issue as in gtk4: I ran a quick search/replace for all usages of @pointer = LibgGObject.new_with_properties, now they all go

def initialize
      @pointer = LibGObject.new_with_properties(LibGtk._gtk_accel_map_get_type, 0, nil, nil).as(Void*)
      self.ref_sink if self.floating?
    end

but the gtk warnings persist.