Created attachment 70343 [details]
app.c

gcc `pkg-config --libs --cflags glib-2.0 gmodule-2.0 gobject-2.0` -g -Wall -Werror -o app app.c

Created attachment 70344 [details]
mod.c

gcc `pkg-config --libs --cflags glib-2.0 gmodule-2.0 gobject-2.0` -g -Wall -Werror -shared -o libmod.so mod.c

Just to be clear, the output of this is:

$ ./app
Load
Parent init
parent dispose
Unload
Load
Parent init
Child init
Child dispose
parent dispose

The expected output is the very same thing, but with an 'Unload' at the end, to tell me that the plugin unloaded after the disposal of the instance of the derived type.

(glib 2.10.3, latest in Ubuntu Dapper 6.06)

Seems to be a "trivial" leak:

--- gobject/gtype.c     2006-08-07 12:27:39.000000000 +0100
+++ gobject/,gtype.c    2006-08-07 12:27:34.000000000 +0100
@@ -2389,6 +2389,13 @@
        }
       
       type_class_init_Wm (node, pclass);
+
+      if (pclass)
+       {
+         G_WRITE_UNLOCK (&type_rw_lock);
+         g_type_class_unref (ptype);
+         G_WRITE_LOCK (&type_rw_lock);
+       }
     }
   G_WRITE_UNLOCK (&type_rw_lock);
   

Though it helps if I post the right patch:

--- gobject/gtype.c     2006-08-07 12:31:53.000000000 +0100
+++ gobject/,gtype.c    2006-08-07 12:32:30.000000000 +0100
@@ -2389,6 +2389,13 @@
        }
       
       type_class_init_Wm (node, pclass);
+
+      if (pclass)
+       {
+         G_WRITE_UNLOCK (&type_rw_lock);
+         g_type_class_unref (pclass);
+         G_WRITE_LOCK (&type_rw_lock);
+       }
     }
   G_WRITE_UNLOCK (&type_rw_lock);
   

But you can't unref the parent type while the child class is still in
use, can you? think about the normal pattern to "chain up".

In a quick look there does seem to be appropriate code to unref the parent type
at the end of type_data_last_unref_Wm(), though there might be something
off about the code in detail.

Yes I was troubled....

For the first instance of the class a reference is taken to its parent class (and that indeed is the reference dropped by type_data_last_unref_Wm).

Inside type_class_init_Wm all data is copy from the parent class and no reference is taken (or required). I guessed that the class_ref here was just an instantiation purely so that class was complete for the copy and could be dropped immediately after.

No harm seems to come of adding that g_type_class_unref...

If it helps, I have this trace for ./app:
==23577== Object at 0x8050090 [GType::Parent, size 40], references 1 [++2, 1--].
==23577== Referenced 2 times, suppressed 0 ...
==23577== Created at:
==23577==    at 0x40425DE: type_data_make_W (gtype.c:1026)
==23577==    by 0x4042D7C: type_data_ref_Wm (gtype.c:1090)
==23577==    by 0x4046A76: g_type_class_ref (gtype.c:1074)
==23577==    by 0x402CFE1: g_object_newv (gobject.c:835)
==23577==    by 0x402D39E: g_object_new_valist (gobject.c:962)
==23577==    by 0x402D54F: g_object_new (gobject.c:800)
==23577==    by 0x8048C83: main (app.c:71)
==23577== ---
==23577== Referenced 1x:
==23577==    at 0x40469BA: g_type_class_ref (gtype.c:1100)
==23577==    by 0x4047111: g_type_class_ref (gtype.c:2404)
==23577==    by 0x402CFE1: g_object_newv (gobject.c:835)
==23577==    by 0x402D39E: g_object_new_valist (gobject.c:962)
==23577==    by 0x402D54F: g_object_new (gobject.c:800)
==23577==    by 0x8048C83: main (app.c:71)
==23577== ---
==23577== Unreferenced 1 times, suppressed 0 ...
==23577== Unreferenced 1x:
==23577==    at 0x40458AF: type_data_last_unref_Wm (gtype.c:1110)
==23577==    by 0x40463EB: g_type_class_unref (gtype.c:1122)
==23577==    by 0x404655A: g_type_free_instance (gtype.c:1621)
==23577==    by 0x4029495: g_object_unref (gobject.c:1797)
==23577==    by 0x8048CC8: main (app.c:74)
==23577== ---
which prompted the patch.

I just looked into this, and I think Chris is right. g_type_class_ref() takes a ref to pclass with which it sets up the derived class (type_class_init_Wm()), but it never unrefs it. This is not the same reference as the one that each type has to its parent (for chaining up, as Owen mentions), which is taken in type_data_ref_Wm(), not in g_type_class_ref() directly. Given this, Chris' patch looks to be correct to me.

Owen, could this patch be considered for applying? (And thanks for looking at this, Chris!)

OK, so actually some more info. I've been running with this, and it generally works fine. I've seen one problem, and would like some comment on it. Once over the past month, I've seen the following g_error() be triggered, implying a race condition in type registration / freeing for GTypeModule-associated GTypes:

GLib-GObject-ERROR **: g_type_plugin_*(0x25fb1c0) invalidly modified type `<name of my type>'

I've been able to trigger this experimentally in a setup where I run 2 threads, each of which has the capability to load/unload a certain module. Is GTypeModule threadsafe?

(Backtrace is being worked on...)

And just now I want to, it won't reproduce under my test conditions anymore. I was using an old glib when I was having those issues (and could reproduce it) and currently svn, would that make a difference? Any patch that would've addressed it?

Sorry for the spam et al, just trying to be helpful. :-).

Ping? This bug is 1 1/2 year old and has a tested patch, please do something. :-).

Tim, can I get you to say yes or no to the patch ?

Ping, 2 1/2 years now...

I'm having a similar or maybe even the same problem. I have an application with two plugins registering types dynamically. The application itself runs two or more threads. These threads create objects of the dynamically registered types quite often. I'd expect to be able to do that but it seems to cause race conditions inside GObject.

Sometimes I get the "invalidly modified type" error described above and sometimes I get this:

GLib-GObject:ERROR:gtype.c:1135:type_data_ref_Wm: assertion failed: (node->data->common.ref_count > 0)

Here's a backtrace for this (without GLib line numbers unfortunately):

+ Trace 216011

The suggested patch doesn't solve the "node->data->common.ref_count > 0" error. Just tested it. Is it a different issue then?

The patch fixes a different issue. The warning I gave above is something I saw once while testing the patch - similar conditions as you. You're basically confirming that my patch is not the cause of the issue, which is what I sort-of thought already.

Speaking of... Can we get this patch in please? It's been sooooo long.

(In reply to comment #6)
> But you can't unref the parent type while the child class is still in
> use, can you? think about the normal pattern to "chain up".

This is right, parent classes need to stay referenced while children exist.

> In a quick look there does seem to be appropriate code to unref the parent type
> at the end of type_data_last_unref_Wm(), though there might be something
> off about the code in detail.

Correct, this is however the reference count acquired by
  type_data_ref_Wm(): if (pnode) type_data_ref_Wm (pnode);

(In reply to comment #8)
> I just looked into this, and I think Chris is right. g_type_class_ref() takes a
> ref to pclass with which it sets up the derived class (type_class_init_Wm()),
> but it never unrefs it.

Yes, thanks for spotting. The patch I'm going to attach fixes this.

(In reply to comment #9)
> I've been able to trigger this experimentally in a setup where I run 2 threads,
> each of which has the capability to load/unload a certain module. Is
> GTypeModule threadsafe?

We have had quite some threadsafety fixes in the past 24 months in the obejct/type system, so very likely you have been hitting another bug with your setup.

Created attachment 145419 [details] [review]
Fix parent class leak during derived class initialization

This is a slightly improved version of Chris' original fix.
Cody, could you please take care of testing this patch with the provided test cases and applying it?

Pushed as 8eebc189440693922e23298a761189cfbe71f796

Comment on attachment 145419 [details] [review]
Fix parent class leak during derived class initialization

Pushed as 8eebc189440693922e23298a761189cfbe71f796