Bug 3801 - Manage Container subclasses are incorrectly resurrected, then leak
Summary: Manage Container subclasses are incorrectly resurrected, then leak
Status: NEW
Alias: None
Product: Gtk#
Classification: Mono
Component: gtk-sharp ()
Version: 2.x
Hardware: PC Windows
: --- normal
Target Milestone: ---
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2012-03-07 18:54 UTC by Mikayla Hutchinson [MSFT]
Modified: 2012-10-13 09:21 UTC (History)
3 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report for Bug 3801 on GitHub or Developer Community if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: GitHub Markdown or Developer Community HTML
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:
Status:
NEW

Description Mikayla Hutchinson [MSFT] 2012-03-07 18:54:50 UTC
After managed subclasses of Gtk.Container are collected, they are resurrected when the object is freed and the container Forall callback is invoked.

This is broken because the resurrected object no longer has the list of children, so cannot iterate over them.

It's doubly broken, because GTK# seems to never release the GCHandle on the ToggleRef for the resurrected object, so the managed and native objects are leaked.

This hits MonoDevelop for 2-5 instances every time a text editor tab is opened and closed. It's not clear how big this leak is, but it may add up to a lot over time.

If I remove the IntPtr ctor - which is only used to resurrect the object - then we can see the stack at the time of resurrection:
glib-sharp.DLL!GLib.MissingIntPtrCtorException.MissingIntPtrCtorException(string msg)
glib-sharp.DLL!GLib.ObjectManager.CreateObject(System.IntPtr raw)
glib-sharp.DLL!GLib.Object.GetObject(System.IntPtr o, bool owned_ref)
gtk-sharp.DLL!Gtk.Container.Forall_cb(System.IntPtr container, bool include_internals, System.IntPtr cb, System.IntPtr data)
[Native code]
glib-sharp.DLL!GLib.ToggleRef.Free()
glib-sharp.DLL!GLib.Object.PerformQueuedUnrefs()
glib-sharp.DLL!GLib.Timeout.TimeoutProxy.Handler()
Comment 1 Mikayla Hutchinson [MSFT] 2012-03-07 20:14:12 UTC
According to the memory profiler, the leaked object is retained via a ToggleRef that is held by a GCHandle. The GCHandle appears to never be freed.
Comment 2 Mikayla Hutchinson [MSFT] 2012-03-09 17:15:18 UTC
Native trace is :

9   ???                           	0x137e960c 0 + 327063052
10  ???                           	0x137e95d4 0 + 327062996
11  ???                           	0x1318b029 0 + 320385065
12  mono                          	0x0000d612 mono_jit_runtime_invoke + 722
13  mono                          	0x0016d28e mono_runtime_invoke + 126
14  mono                          	0x00173950 mono_runtime_invoke_array + 1296
15  mono                          	0x001189f2 ves_icall_InternalInvoke + 1090
16  ???                           	0x055ec828 0 + 90097704
17  ???                           	0x055ec074 0 + 90095732
18  ???                           	0x055ebebd 0 + 90095293
19  ???                           	0x055ea059 0 + 90087513
20  ???                           	0x055e9dbc 0 + 90086844
21  ???                           	0x028eff70 0 + 42925936
22  ???                           	0x028efdac 0 + 42925484
23  ???                           	0x0ce3e114 0 + 216260884
24  ???                           	0x0bbc763c 0 + 196900412
25  libgtk-quartz-2.0.0.dylib     	0x04a1f528 gtk_container_foreach + 280
26  libgtk-quartz-2.0.0.dylib     	0x04a1dfe8 gtk_container_destroy + 152
27  libgobject-2.0.0.dylib        	0x04973eff g_cclosure_marshal_VOID__VOID + 223
28  libgobject-2.0.0.dylib        	0x0495554e g_type_class_meta_marshal + 142
29  libgobject-2.0.0.dylib        	0x04955154 g_closure_invoke + 468
30  libgobject-2.0.0.dylib        	0x049738d6 signal_emit_unlocked_R + 5078
31  libgobject-2.0.0.dylib        	0x04971e0b g_signal_emit_valist + 2619
32  libgobject-2.0.0.dylib        	0x049722d1 g_signal_emit + 65
33  libgtk-quartz-2.0.0.dylib     	0x04b19698 gtk_object_dispose + 88
34  libgtk-quartz-2.0.0.dylib     	0x04c92cc9 gtk_widget_dispose + 153
35  libgobject-2.0.0.dylib        	0x0495da46 g_object_unref + 358
36  libgobject-2.0.0.dylib        	0x0495d74d g_object_remove_toggle_ref + 717
37  ???                           	0x0bbcadb8 0 + 196914616
38  ???                           	0x0bbcad10 0 + 196914448
39  ???                           	0x0cfa9df8 0 + 217751032
40  ???                           	0x0cfa8d8a 0 + 217746826
41  ???                           	0x06fb0a5c 0 + 117115484
42  libglib-2.0.0.dylib           	0x04845186 g_timeout_dispatch + 102
43  libglib-2.0.0.dylib           	0x0484126e g_main_dispatch + 510
44  libglib-2.0.0.dylib           	0x04842afb g_main_context_dispatch + 155
45  libglib-2.0.0.dylib           	0x048430a3 g_main_context_iterate + 1331
46  libglib-2.0.0.dylib           	0x0484333e g_main_context_iteration + 190
47  libgtk-quartz-2.0.0.dylib     	0x04ada572 gtk_main_iteration_do + 66
Comment 3 Mikayla Hutchinson [MSFT] 2012-03-09 17:42:50 UTC
The container has already been destroyed before gtk_object_dispose destroys it again. In theory this is fine, because it's supposed to be safe to destroy multiple times, but that doesn't seem to be something gtk# copes with well. GTK# should be retaining the object while freeing the toggleref. A possible viable workaround might be to skip the destroy if the object's already been destroyed.
Comment 4 Mikayla Hutchinson [MSFT] 2012-03-09 21:21:27 UTC
The following code snippet illustrates the problem.It creates and destroys 10 container subclasses. Then, an idle handler pumps the GC and prints the number of created, re-wrapped and collected objects. We would expect:
    cr 10 wr 0 co 10 net 0
because there is no point in resurfacing native backing objects for managed subclasses -- all managed state has gone.

What we get instead is  wrappers being created and collected continually, typically something like:
cr 10 wr 5652 co 5662 net 0

The problem is that the class overrides ForAll, which is required for container subclasses to do anything useful. This causes GTK# to install a native callback that unwraps the handle to a managed object and calls its forall override. So far so good. The problem is:
1) managed container gets collected
2) GTK# queues a g_object_remove_toggle_ref on a timeout
3) The last reference is released, so the native gobject is disposed 
4) gtk_object_dispose destroys the object even if it was already destroyed, to ensured it's cleaned up - it's supposed to be safe to destroy objects multiple times
5) gtk_container_destroy destroys its children via forall
6) The GTK# forall native callback tries to unwrap the object, creates a new wrapper, wrapper references the native object
7) wrapper is collected, GOTO 2

Here is the code that demonstrates the problem:

	class MainClass
	{
		public static void Main (string[] args)
		{
			Application.Init ();
			
			CreateObjects ();
			
			GLib.Idle.Add (delegate {
				GC.Collect ();
				Console.WriteLine ("cr {0} wr {1} co {2} net {3}",
					C.created, C.wrapped, C.collected,
					(C.created + C.wrapped) - C.collected);
				return true;
			});
			Application.Run ();
		}

		static void CreateObjects ()
		{
			for (int i = 0; i < 10; i++) {
				var c = new C ();
				c.Destroy ();
			}
		}
	}
	
	class C : Container	
	{
		public static int collected, wrapped, created;
		
		protected override void ForAll (bool include_internals, Callback callback)
		{
			Console.WriteLine ("bar");
		}
		
		public C ()
		{
			created++;
		}
		
		protected C (IntPtr handle)
		{
			wrapped++;
		}
		
		~C ()
		{
			collected++;
		}
	}
Comment 5 Mikayla Hutchinson [MSFT] 2012-03-09 22:11:21 UTC
A potential fix is at http://mjhutchinson.com/files/temp/containerfix.diff.
Comment 6 Mikayla Hutchinson [MSFT] 2012-03-13 19:07:58 UTC
MD patch here: http://mjhutchinson.com/files/temp/fix-container-leak.diff
Comment 7 Mikayla Hutchinson [MSFT] 2012-03-13 21:37:22 UTC
The patch fails on Windows - Dynamic methods that p/invoke must be associated with a module.
Comment 8 Mikayla Hutchinson [MSFT] 2012-03-28 19:47:52 UTC
Committed the workaround to MD a few days ago. GTK# still needs fixing.
Comment 9 Andres G. Aragoneses 2012-09-10 05:01:12 UTC
Just saw this from https://github.com/mono/xwt/commit/3fd54f03241c1c35f23c7b47c58933f2d7bf4423 , good work Michael. I wonder if Banshee is affected by this leak as well...
Comment 10 Mikayla Hutchinson [MSFT] 2012-09-10 12:27:40 UTC
Yes, it will be happening to banshee too.
Comment 11 Andres G. Aragoneses 2012-09-10 12:48:05 UTC
Had a quick look and the only classes where we inherit from Container are AnimatedWidget, MenuButton, and AnimatedBox (latter is abstract so potentially affecting to lot more).

(And didn't look into hyena yet though...)
Comment 12 Bertrand Lorentz 2012-10-13 09:21:31 UTC
The fix for this was merged into GTK#, both in the gtk-sharp-2.12 branch and master:
https://github.com/mono/gtk-sharp/pull/50
https://github.com/mono/gtk-sharp/pull/51

So I think this can be closed.

FYI, to verify the fix on git master, the sample code above has to be modified, as Glib.Object now uses the Dispose pattern:
This line
 ~C ()
should be replaced with
protected override void Dispose (bool disposing)