glib2: stop to Ruby's Hash to keep relative objects

GitHub: fix #1162

Reported by Izumi Tsutsui. Thanks!!!

If we use Ruby's Hash for it, we need to change Ruby's Hash on freeing
GObject objects. It's not a good GC manner.

G_CHILD_REMOVE_ALL() is deprecated. Use
rbgobj_object_remove_relatives() instead.

glib2/ext/glib2/glib2.def

@@ -61,6 +61,9 @@ EXPORTS
 	rbgobj_class_init_func
 	rbgobj_register_type
 	rbgobj_object_alloc_func
+	rbgobj_object_add_relative
+	rbgobj_object_remove_relative
+	rbgobj_object_remove_relatives
 	rbgobj_set_signal_func
 	rbgobj_get_signal_func
 	rbgobj_set_signal_call_func

glib2/ext/glib2/rbgobj_closure.c

@@ -1,6 +1,6 @@
 /* -*- c-file-style: "ruby"; indent-tabs-mode: nil -*- */
 /*
- *  Copyright (C) 2011-2016  Ruby-GNOME2 Project Team
+ *  Copyright (C) 2011-2018  Ruby-GNOME2 Project Team
  *  Copyright (C) 2002-2006  Ruby-GNOME2 Project
  *  Copyright (C) 2002,2003  Masahiro Sakai
  *
@@ -191,8 +191,9 @@ rclosure_invalidate(G_GNUC_UNUSED gpointer data, GClosure *closure)
         for (next = rclosure->objects; next; next = next->next) {
             GObject *object = G_OBJECT(next->data);
             VALUE obj = rbgobj_ruby_object_from_instance2(object, FALSE);
-            if (!NIL_P(rclosure->rb_holder) && !NIL_P(obj))
-                G_REMOVE_RELATIVE(obj, id_closures, rclosure->rb_holder);
+            if (!NIL_P(rclosure->rb_holder) && !NIL_P(obj)) {
+                rbgobj_object_remove_relative(obj, rclosure->rb_holder);
+            }
         }
 
         rclosure_unref(rclosure);
@@ -286,16 +287,15 @@ rclosure_weak_notify(gpointer data, GObject* where_the_object_was)
 void
 g_rclosure_attach(GClosure *closure, VALUE object)
 {
-    static VALUE mGLibObject = (VALUE)NULL;
+    static VALUE cGLibObject = Qnil;
     GRClosure *rclosure = (GRClosure *)closure;
 
-    G_RELATIVE2(object, Qnil, id_closures, rclosure->rb_holder);
-
-    if (!mGLibObject) {
-        mGLibObject = rb_const_get(mGLib, rb_intern("Object"));
+    if (NIL_P(cGLibObject)) {
+        cGLibObject = rb_const_get(mGLib, rb_intern("Object"));
     }
-    if (rb_obj_is_kind_of(object, mGLibObject)) {
+    if (rb_obj_is_kind_of(object, cGLibObject)) {
         GObject *gobject;
+        rbgobj_object_add_relative(object, rclosure->rb_holder);
         gobject = RVAL2GOBJ(object);
         rclosure->count++;
         g_object_weak_ref(gobject, rclosure_weak_notify, rclosure);

glib2/ext/glib2/rbgobj_object.c

@@ -43,19 +43,32 @@ weak_notify(gpointer data, G_GNUC_UNUSED GObject *where_the_object_was)
     gobj_holder *holder = data;
 
     rbgobj_instance_call_cinfo_free(holder->gobj);
-    rbgobj_invalidate_relatives(holder->self);
+    g_hash_table_unref(holder->rb_relatives);
     holder->destroyed = TRUE;
 
     g_object_unref(holder->gobj);
     holder->gobj = NULL;
 }
 
+static void
+holder_relatives_mark(gpointer key, gpointer value, gpointer user_data)
+{
+    VALUE rb_relative = (VALUE)value;
+    rb_gc_mark(rb_relative);
+}
+
 static void
 holder_mark(void *data)
 {
     gobj_holder *holder = data;
-    if (holder->gobj && !holder->destroyed)
-        rbgobj_instance_call_cinfo_mark(holder->gobj);
+
+    if (!holder->gobj)
+        return;
+    if (holder->destroyed)
+        return;
+
+    rbgobj_instance_call_cinfo_mark(holder->gobj);
+    g_hash_table_foreach(holder->rb_relatives, holder_relatives_mark, NULL);
 }
 
 static void
@@ -91,6 +104,55 @@ static const rb_data_type_t rg_glib_object_type = {
     RUBY_TYPED_FREE_IMMEDIATELY,
 };
 
+void
+rbgobj_object_add_relative(VALUE rb_gobject, VALUE rb_relative)
+{
+    gobj_holder *holder;
+    TypedData_Get_Struct(rb_gobject,
+                         gobj_holder,
+                         &rg_glib_object_type,
+                         holder);
+    g_hash_table_insert(holder->rb_relatives,
+                        (gpointer)(rb_relative),
+                        (gpointer)(rb_relative));
+}
+
+void
+rbgobj_object_remove_relative(VALUE rb_gobject, VALUE rb_relative)
+{
+    gobj_holder *holder;
+    TypedData_Get_Struct(rb_gobject,
+                         gobj_holder,
+                         &rg_glib_object_type,
+                         holder);
+    g_hash_table_remove(holder->rb_relatives,
+                        (gpointer)(rb_relative));
+}
+
+static gboolean
+rbgobj_object_remove_relatives_body(gpointer key,
+                                    gpointer value,
+                                    gpointer user_data)
+{
+    VALUE rb_relative = (VALUE)value;
+    VALUE rb_relative_class = (VALUE)user_data;
+
+    return RVAL2CBOOL(rb_obj_is_kind_of(rb_relative, rb_relative_class));
+}
+
+void
+rbgobj_object_remove_relatives(VALUE rb_gobject, VALUE rb_relative_class)
+{
+    gobj_holder *holder;
+    TypedData_Get_Struct(rb_gobject,
+                         gobj_holder,
+                         &rg_glib_object_type,
+                         holder);
+    g_hash_table_foreach_remove(holder->rb_relatives,
+                                rbgobj_object_remove_relatives_body,
+                                (gpointer)(rb_relative_class));
+}
+
 VALUE
 rbgobj_object_alloc_func(VALUE klass)
 {
@@ -105,6 +167,7 @@ rbgobj_object_alloc_func(VALUE klass)
     holder->gobj  = NULL;
     holder->cinfo = NULL;
     holder->destroyed = FALSE;
+    holder->rb_relatives = g_hash_table_new(g_direct_hash, g_direct_equal);
 
     return result;
 }

glib2/ext/glib2/rbgobject.c

@@ -1,6 +1,6 @@
 /* -*- c-file-style: "ruby"; indent-tabs-mode: nil -*- */
 /*
- *  Copyright (C) 2011  Ruby-GNOME2 Project Team
+ *  Copyright (C) 2011-2018  Ruby-GNOME2 Project Team
  *  Copyright (C) 2003-2006  Ruby-GNOME2 Project Team
  *  Copyright (C) 2002,2003  Masahiro Sakai
  *  Copyright (C) 1998-2000 Yukihiro Matsumoto,
@@ -183,16 +183,25 @@ rbgobj_ruby_object_from_instance_with_unref(gpointer instance)
 void
 rbgobj_add_relative(VALUE obj, VALUE relative)
 {
-    VALUE hash = Qnil;
+    static VALUE mGLibObject = Qnil;
+    if (NIL_P(mGLibObject)) {
+        mGLibObject = rb_const_get(mGLib, rb_intern("Object"));
+    }
 
-    if (RVAL2CBOOL(rb_ivar_defined(obj, id_relatives)))
-        hash = rb_ivar_get(obj, id_relatives);
+    if (rb_obj_is_kind_of(obj, mGLibObject)) {
+        rbgobj_object_add_relative(obj, relative);
+    } else {
+        VALUE hash = Qnil;
 
-    if (NIL_P(hash) || TYPE(hash) != RUBY_T_HASH) {
-        hash = rb_hash_new();
-        rb_ivar_set(obj, id_relatives, hash);
+        if (RVAL2CBOOL(rb_ivar_defined(obj, id_relatives)))
+            hash = rb_ivar_get(obj, id_relatives);
+
+        if (NIL_P(hash) || TYPE(hash) != RUBY_T_HASH) {
+            hash = rb_hash_new();
+            rb_ivar_set(obj, id_relatives, hash);
+        }
+        rb_hash_aset(hash, relative, Qnil);
     }
-    rb_hash_aset(hash, relative, Qnil);
 }
 
 void
@@ -207,16 +216,26 @@ rbgobj_invalidate_relatives(VALUE obj)
 void
 rbgobj_add_relative_removable(VALUE obj, VALUE relative, ID obj_ivar_id, VALUE hash_key)
 {
-    VALUE hash = Qnil;
+    static VALUE cGLibObject = Qnil;
+    if (NIL_P(cGLibObject)) {
+        cGLibObject = rb_const_get(mGLib, rb_intern("Object"));
+    }
 
-    if (RVAL2CBOOL(rb_ivar_defined(obj, obj_ivar_id)))
-        hash = rb_ivar_get(obj, obj_ivar_id);
+    if (obj_ivar_id == rbgobj_id_children &&
+        rb_obj_is_kind_of(obj, cGLibObject)) {
+        rbgobj_object_add_relative(obj, hash_key);
+    } else {
+        VALUE hash = Qnil;
 
-    if (NIL_P(hash) || TYPE(hash) != RUBY_T_HASH) {
-        hash = rb_hash_new();
-        rb_ivar_set(obj, obj_ivar_id, hash);
+        if (RVAL2CBOOL(rb_ivar_defined(obj, obj_ivar_id)))
+            hash = rb_ivar_get(obj, obj_ivar_id);
+
+        if (NIL_P(hash) || TYPE(hash) != RUBY_T_HASH) {
+            hash = rb_hash_new();
+            rb_ivar_set(obj, obj_ivar_id, hash);
+        }
+        rb_hash_aset(hash, hash_key, relative);
     }
-    rb_hash_aset(hash, hash_key, relative);
 }
 
 VALUE
@@ -236,15 +255,25 @@ rbgobj_get_relative_removable(VALUE obj, ID obj_ivar_id, VALUE hash_key)
 void
 rbgobj_remove_relative(VALUE obj, ID obj_ivar_id, VALUE hash_key)
 {
-    VALUE hash = Qnil;
-
-    if (RVAL2CBOOL(rb_ivar_defined(obj, obj_ivar_id)))
-        hash = rb_ivar_get(obj, obj_ivar_id);
+    static VALUE cGLibObject = Qnil;
+    if (NIL_P(cGLibObject)) {
+        cGLibObject = rb_const_get(mGLib, rb_intern("Object"));
+    }
 
-    if (NIL_P(hash) || TYPE(hash) != RUBY_T_HASH) {
-        /* should not happen. */
+    if ((obj_ivar_id == id_relatives || obj_ivar_id == rbgobj_id_children) &&
+        rb_obj_is_kind_of(obj, cGLibObject)) {
+        rbgobj_object_remove_relative(obj, hash_key);
     } else {
-        rb_funcall(hash, id_delete, 1, hash_key);
+        VALUE hash = Qnil;
+
+        if (RVAL2CBOOL(rb_ivar_defined(obj, obj_ivar_id)))
+            hash = rb_ivar_get(obj, obj_ivar_id);
+
+        if (NIL_P(hash) || TYPE(hash) != RUBY_T_HASH) {
+            /* should not happen. */
+        } else {
+            rb_funcall(hash, id_delete, 1, hash_key);
+        }
     }
 }
 

glib2/ext/glib2/rbgobject.h

@@ -79,6 +79,7 @@ RUBY_GLIB2_VAR  ID rbgobj_id_children;
     (rbgobj_add_relative_removable(self, Qnil, rbgobj_id_children, child))
 #define G_CHILD_REMOVE(self, child) \
     (rbgobj_remove_relative(self, rbgobj_id_children, child))
+/* Deprecated since 3.2.5. Use rbobj_object_remove_relatives() instead. */
 #define G_CHILD_REMOVE_ALL(self) \
     (rbgobj_remove_relative_all(self, rbgobj_id_children))
 
@@ -147,6 +148,13 @@ extern VALUE rbgobj_ruby_object_from_instance2(gpointer instance, gboolean alloc
 extern VALUE rbgobj_ruby_object_from_instance_with_unref(gpointer instance);
 extern void rbgobj_instance_unref(gpointer instance);
 
+extern void rbgobj_object_add_relative(VALUE rb_gobject,
+                                       VALUE rb_relative);
+extern void rbgobj_object_remove_relative(VALUE rb_gobject,
+                                          VALUE rb_relative);
+extern void rbgobj_object_remove_relatives(VALUE rb_gobject,
+                                           VALUE rb_relative_class);
+
 extern void rbgobj_add_relative(VALUE obj, VALUE relative);
 extern void rbgobj_invalidate_relatives(VALUE obj);
 extern void rbgobj_add_relative_removable(VALUE obj, VALUE relative,

glib2/ext/glib2/rbgprivate.h

@@ -1,6 +1,6 @@
 /* -*- c-file-style: "ruby"; indent-tabs-mode: nil -*- */
 /*
- *  Copyright (C) 2007-2017  Ruby-GNOME2 Project Team
+ *  Copyright (C) 2007-2018  Ruby-GNOME2 Project Team
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -42,6 +42,7 @@ typedef struct {
     GObject* gobj;
     const RGObjClassInfo* cinfo;
     gboolean destroyed;
+    GHashTable *rb_relatives;
 } gobj_holder;
 
 typedef struct {

gtk2/ext/gtk2/rbgtkliststore.c

@@ -1,6 +1,6 @@
 /* -*- c-file-style: "ruby"; indent-tabs-mode: nil -*- */
 /*
- *  Copyright (C) 2011  Ruby-GNOME2 Project Team
+ *  Copyright (C) 2011-2018  Ruby-GNOME2 Project Team
  *  Copyright (C) 2002-2005 Masao Mutoh
  *
  *  This library is free software; you can redistribute it and/or
@@ -333,7 +333,11 @@ rg_append(VALUE self)
 static VALUE
 rg_clear(VALUE self)
 {
-    G_CHILD_REMOVE_ALL(self);
+    static VALUE rb_cGtkTreeIter = Qnil;
+    if (NIL_P(rb_cGtkTreeIter)) {
+        rb_cGtkTreeIter = GTYPE2CLASS(GTK_TYPE_TREE_ITER);
+    }
+    rbgobj_object_remove_relatives(self, rb_cGtkTreeIter);
     gtk_list_store_clear(_SELF(self));
     return self;
 }

gtk2/ext/gtk2/rbgtktreestore.c

@@ -1,6 +1,6 @@
 /* -*- c-file-style: "ruby"; indent-tabs-mode: nil -*- */
 /*
- *  Copyright (C) 2011-2012  Ruby-GNOME2 Project Team
+ *  Copyright (C) 2011-2018 Ruby-GNOME2 Project Team
  *  Copyright (C) 2002-2006 Masao Mutoh
  *
  *  This library is free software; you can redistribute it and/or
@@ -273,7 +273,11 @@ rg_iter_depth(VALUE self, VALUE iter)
 static VALUE
 rg_clear(VALUE self)
 {
-    G_CHILD_REMOVE_ALL(self);
+    static VALUE rb_cGtkTreeIter = Qnil;
+    if (NIL_P(rb_cGtkTreeIter)) {
+        rb_cGtkTreeIter = GTYPE2CLASS(GTK_TYPE_TREE_ITER);
+    }
+    rbgobj_object_remove_relatives(self, rb_cGtkTreeIter);
     gtk_tree_store_clear(_SELF(self));
     return self;
 }

gtk2/ext/gtk2/rbgtktreeviewcolumn.c

@@ -1,6 +1,6 @@
 /* -*- c-file-style: "ruby"; indent-tabs-mode: nil -*- */
 /*
- *  Copyright (C) 2011  Ruby-GNOME2 Project Team
+ *  Copyright (C) 2011-2018 Ruby-GNOME2 Project Team
  *  Copyright (C) 2002-2004 Masao Mutoh
  *
  *  This library is free software; you can redistribute it and/or
@@ -83,7 +83,11 @@ rg_pack_end(VALUE self, VALUE cell, VALUE expand)
 static VALUE
 rg_clear(VALUE self)
 {
-    G_CHILD_REMOVE_ALL(self);
+    static VALUE rb_cGtkCellRenderer = Qnil;
+    if (NIL_P(rb_cGtkCellRenderer)) {
+        rb_cGtkCellRenderer = GTYPE2CLASS(GTK_TYPE_CELL_RENDERER);
+    }
+    rbgobj_object_remove_relatives(self, rb_cGtkCellRenderer);
     gtk_tree_view_column_clear(_SELF(self));
     return self;
 }