Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak when marshalling Delegate to native code #6161

Open
ggaller opened this issue Dec 5, 2017 · 8 comments
Open

Memory leak when marshalling Delegate to native code #6161

ggaller opened this issue Dec 5, 2017 · 8 comments
Assignees

Comments

@ggaller
Copy link

ggaller commented Dec 5, 2017

In our project we send callback delegate to native code and found a memory leak at runtime.
This is a sample project that reproduce the problem.

{
   [UnmanagedFunctionPointer( CallingConvention.Cdecl )]
   internal delegate void NativeFunctionCallback(int arg);

   [DllImport("libnative", CallingConvention = CallingConvention.Cdecl)]
   internal static extern void native_function(int arg, NativeFunctionCallback cb);

   private NativeFunctionCallback _cb;

   public CallbackOwner()
   {
      _cb = CallbackImplementation;
   }

   public void Run(int n)
   {
      native_function( n, _cb ); // Expecting leak here!
   }

   private void CallbackImplementation( int arg )
   {
      Console.WriteLine( arg );
   }
}

And write some loop to test

int n = 0;
while ( true )
{
   new CallbackOwner().Run( ++n );

   // Collect managed memory.
   if (n%1000 == 0)
      GC.Collect();
}

"libnative" is a simple test programm that only execute callback.

Full test project added to attachement.
This leak reproduced on Mac and Linux on Windows all fine. With mono-boehm leak not reproduced, only with SGen.

Profiler show memory leak at:

Leak: 0x7f81e74001c0 size=16 zone: DefaultMallocZone_0x1068f1000
0x03b0600e 0x00000000 0x00000000 0x00000000 .`..............
Call stack: [thread 0x7fffb321e340]: | start | main | mono_main | mono_jit_exec | do_exec_main_checked | do_runtime_invoke | mono_jit_runtime_invoke | 0x106a0d427 | mono_delegate_to_ftnptr | mono_jit_compile_method_with_opt | mono_jit_compile_method_inner | mini_method_compile | mono_save_seq_point_info | mono_seq_point_info_new | monoeg_malloc0 | calloc | malloc_zone_calloc

We really like the Mono project and enjoy using it on our servers, but because of the leaks we have to restart the server every 24 hours.

Please help us :)

leaks_mono-sgen.zip
MonoPInvokeLeak2.zip

@kumpera
Copy link
Contributor

kumpera commented Dec 5, 2017

Hi, thanks for the test case.
What version of mono are you using? I think this was recently fixed.

@ggaller
Copy link
Author

ggaller commented Dec 6, 2017

Mono JIT compiler version 5.4.1.6 (tarball Wed Nov 8 16:59:44 UTC 2017)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
TLS: __thread
SIGSEGV: altstack
Notifications: epoll
Architecture: amd64
Disabled: none
Misc: softdebug
LLVM: supported, not enabled.
GC: sgen (concurrent by default)

@ggaller
Copy link
Author

ggaller commented Dec 6, 2017

Looks like previously reported leaks of 5.4 were fixed by 4cf2b5f#diff-889eecc0e02f623fc21d69e97f0983c4

Profiler not show memory leaks, but memory of the sample application attached here still is growing rapidly after our test on 5.11. Looks like stub functions generated for delegates marshaling are never deleted from memory (mono/metadata/marshal.c: mono_delegate_to_ftnptr()).
Is it by design?

@kumpera
Copy link
Contributor

kumpera commented Dec 6, 2017

Memory is released when the delegate is collected.

@kumpera
Copy link
Contributor

kumpera commented Dec 6, 2017

I can repro the leak. I'll take a look at it.
Thanks again for the test case!

@neoxic
Copy link

neoxic commented Dec 9, 2017

I can confirm this leak too (Mono 5.0). Waiting eagerly for a fix... Thanks!

@kumpera
Copy link
Contributor

kumpera commented Dec 11, 2017

This is a leak in mono_delegate_to_ftnptr when freeing the native MonoMethod signature. It happens because too much sharing of MonoType's that triggers memory corruption and thus we addressed that by not freeing the signatures.

As a workaround, use delegates to static methods, which don't suffer from this issue and have much superior performance.

@neoxic
Copy link

neoxic commented Dec 11, 2017

Thanks, Rogrigo! So this bug was introduced by trying to conceal another bug, right? Are there plans to address the underlying issue to break this vicious cycle?

Static delegates will require to explicitly pass this through P/Invoke calls type casting it on the way. This will in turn reduce type safety and can potentially introduce subtle hard-to-find bugs. Might work as a temporary workaround of course, but hardly as a long standing solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants