Bug 605295

Summary: Memory leak with DynamicMethod and --debug
Product: [Mono] Mono: Runtime Reporter: Lluis Sanchez <lluis>
Component: JITAssignee: Paolo Molaro <lupus>
Status: RESOLVED FIXED QA Contact: Mono Bugs <mono-bugs>
Severity: Normal    
Priority: P5 - None CC: forgotten_vxPDddArjq
Version: SVN   
Target Milestone: ---   
Hardware: Other   
OS: openSUSE 11.3   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Bug Depends on:    
Bug Blocks: 731579    
Attachments: Testcase
fix some leaks in the debug code / xdebug code
New test case
revert some patches

Description Lluis Sanchez 2010-05-12 18:39:53 UTC
This test case makes memory grow quickly. It works fine if the instance delegate (cls.Run) is replaced by a delegate to a static method.

using System;
using System.Runtime.InteropServices;

namespace DelegateTest
{
	class MainClass
	{
		public static void Main (string[] args)
		{
			MainClass cls = new MainClass ();
			while (true) {
				uint id = g_timeout_add (500, cls.Run, IntPtr.Zero);
				g_source_remove (id);
			}
		}
		
		bool Run ()
		{
			return false;
		}
		
		[DllImport("libglib-2.0.so")]
		static extern void g_source_remove (uint id);
		
		[DllImport("libglib-2.0.so")]
		static extern uint g_timeout_add (uint id, Func<bool> function, IntPtr data);
	}
}
Comment 1 Alan McGovern 2010-05-12 22:01:03 UTC
This does not leak for me on a 64bit core2duo with mono 2.6.4. However with r156824 of mono/mcs it leaks like a sieve.
Comment 2 Forgotten User vxPDddArjq 2010-05-12 23:09:09 UTC
Can't reproduce this with svn head. Could it be a glib leak ?
Comment 3 Rodrigo Kumpera 2010-05-12 23:22:30 UTC
I can repro on OS 11.2 32 bits with sgen.
Comment 4 Alan McGovern 2010-05-12 23:26:51 UTC
Created attachment 361890 [details]
Testcase

Attached a slightly nicer testcase which tests static and instance delegates. The only case which leaks for me on r156824 is case 4 where we keep recreating a Func<bool> to an instance method. Every other case has more or less constant memory usage. This is on x64.
Comment 5 Lluis Sanchez 2010-05-13 00:08:32 UTC
Some new info: I can reproduce the leak (using my test case) only when running with the --debug option. Without that option, it works fine. I'm on linux x86.
Comment 6 Alan McGovern 2010-05-15 19:22:40 UTC
Yet more info!

The only leaking subtest is the "non-cached instance delegate" and it only leaks under --debug or when MONO_XDEBUG is set. Moonlight sets debug and i always run with MONO_XDEBUG.
Comment 7 Forgotten User vxPDddArjq 2010-05-15 19:35:43 UTC
This happens because these are dynamic methods, and we don't free their debugging info when the method is freed. We could, anyway, because most of it is allocated from mempool like structures in mono-debug.c. A solution would be disabling the generation of debug info for these methods.
Comment 8 Forgotten User vxPDddArjq 2010-05-15 19:36:11 UTC
I meant: 'We couldn't, anyway'.
Comment 9 Geoff Norton 2010-05-15 21:24:27 UTC
Created attachment 362494 [details]
fix some leaks in the debug code / xdebug code

Running the loop 10000 times with patch:

==14217== LEAK SUMMARY:
==14217==    definitely lost: 366 bytes in 26 blocks
==14217==    indirectly lost: 330 bytes in 33 blocks
==14217==      possibly lost: 126,354 bytes in 386 blocks
==14217==    still reachable: 3,958,669 bytes in 5,237 blocks

Running the loop 10000 on trunk:

==16425== LEAK SUMMARY:
==16425==    definitely lost: 3,628,469 bytes in 28,511 blocks
==16425==    indirectly lost: 1,983,495 bytes in 21,943 blocks
==16425==      possibly lost: 127,618 bytes in 390 blocks
==16425==    still reachable: 3,943,685 bytes in 4,747 blocks
Comment 10 Geoff Norton 2010-05-15 21:27:44 UTC
We should probably add a mono_debug_symfile_free_location tho instead of directly freeing the source_file parameter.
Comment 11 Forgotten User vxPDddArjq 2010-05-15 21:38:31 UTC
Looks ok.
Comment 12 Geoff Norton 2010-05-16 01:16:16 UTC
Fixed the low hanging fruit in r157388, r157389, r157390.

Should we leave this bug open for the DM debug info mempool leak, or is there another better bug for that?
Comment 13 Rodrigo Kumpera 2010-05-16 15:47:04 UTC
We suck at keeping --debug healthy, but we're are supposed to.

So let's keep this one open.
Comment 14 Mike Krueger 2010-05-17 08:04:20 UTC
*** Bug 605247 has been marked as a duplicate of this bug. ***
Comment 15 Lluis Sanchez 2010-05-18 18:27:28 UTC
Created attachment 363024 [details]
New test case

The attached test case shows the memory leak even when --debug is not provided.
Comment 16 Geoff Norton 2010-05-18 21:22:27 UTC
The new test doesn't definitively show any leak, you're allocating objects in a while(true) loop which means that objects will just continue to allocate.  Add a System.GC.Collect () to the bottom of the loop and you will see some memory growth that I havn't tracked down yet, but much much slower.

Limiting the loop to 1000 iterations with a GC.Collect gives identical results to my fixed numbers above.
Comment 17 Geoff Norton 2010-05-18 23:57:49 UTC
Ok, we did some debugging on irc, the issue here was introduced in r131613.
Comment 18 Rodrigo Kumpera 2010-05-19 00:44:32 UTC
The problem is that target_loc holds a reference to the delegate target. If the target reference back the delegate, we have a circular reference that prevent things from been collected.

The way to fix it is to use a weak-ref gchandle to target and resolve it on every invocation.

I'll work on it during this week.
Comment 19 Geoff Norton 2010-05-19 01:04:36 UTC
Created attachment 363090 [details]
revert some patches

The attached reversion patch should fix the issue on 2.6 where sgen doesn't really work anyways, so if we want to push a release we should test with this.
Comment 20 Forgotten User vxPDddArjq 2010-05-19 02:34:29 UTC
Its not just about sgen, embedding an object reference into the generated code is problematic.
Comment 21 Rodrigo Kumpera 2010-05-19 19:30:45 UTC
So the plan is to go with 2.8 patch for 2.6. It applies clean and should be just fine.
Comment 22 Rodrigo Kumpera 2010-05-20 00:48:13 UTC
Fixed trunk in r157589. Backporting to 2.6 tomorrow.
Comment 23 Rodrigo Kumpera 2010-05-21 01:01:59 UTC
Backported to 2.6 in r157655.