Skip to content

memory leak - simple function call in a loop #541

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

Closed
den-run-ai opened this issue Sep 11, 2017 · 14 comments
Closed

memory leak - simple function call in a loop #541

den-run-ai opened this issue Sep 11, 2017 · 14 comments
Labels

Comments

@den-run-ai
Copy link
Contributor

den-run-ai commented Sep 11, 2017

I'm getting a memory leak with simple calls from .NET to Python function. The issue is reproducible on all pythonnet versions that I tested. When I test Python calls to .NET, then the memory leak is less obvious, since all resources are released when calling the CPython gc.collect(). Calling the same function from .NET does not release any resources, but calling the CLR GC.WaitForPendingFinalizers() blocks the execution.

memoryleakpykw.zip

>>> import clr
>>> import System
>>> for i in range(20000000):
...   a=System.Double(i)
...
>>> import gc
>>> gc.collect()

pyinclr.py

def testmethod():
	return "abc"*3
using System;
using Python.Runtime;

namespace memoryleakpykw
{
    class Program
    {
        static void Main(string[] args)
        {
            PyObject pymodule;
            PyObject gc;
            PyObject testmethod;

            PythonEngine.Initialize();

            IntPtr pylock = PythonEngine.AcquireLock();
            {
                PyObject sys = PythonEngine.ImportModule("sys");// Py.Import("sys");
                gc = PythonEngine.ImportModule("gc");// Py.Import("sys");
                sys.GetAttr("path").InvokeMethod("append",
                    new PyString(@"C:\Users\denis.akhiyarov\source\repos\memoryleakpykw\pyinclr"));
                pymodule = PythonEngine.ImportModule("pyinclr");
                testmethod = pymodule.GetAttr("testmethod");
            }

            PythonEngine.ReleaseLock(pylock);

            Console.WriteLine("starting test for memory leak, press any key");
            Console.ReadKey();

            pylock = PythonEngine.AcquireLock();

            {
                for (int i=1; i<=1000000; i++)
                {
                    testmethod.Invoke();
                }
            }

            gc.InvokeMethod("collect"); 

            PythonEngine.ReleaseLock(pylock);
            
            GC.Collect();
            //GC.WaitForPendingFinalizers(); // this blocks execution
            GC.Collect();

            Console.WriteLine("finished test for memory leak, press any key");
            Console.ReadKey();
        }

    }
}
@den-run-ai
Copy link
Contributor Author

Actually GC.WaitForPendingFinalizers(); is blocking, not gc.collect!

@dmitriyse
Copy link
Contributor

dmitriyse commented Sep 11, 2017

Please try this version of your code:

using System;
using Python.Runtime;

namespace memoryleakpykw
{
    class Program
    {
        static void Main(string[] args)
        {
            PyObject pymodule;
            PyObject gc;
            PyObject testmethod;

            PythonEngine.Initialize();
            var ts = PythonEngine.BeginAllowThread();

            IntPtr pylock = PythonEngine.AcquireLock();
            {
                PyObject sys = PythonEngine.ImportModule("sys");// Py.Import("sys");
                gc = PythonEngine.ImportModule("gc");// Py.Import("sys");
                sys.GetAttr("path").InvokeMethod("append",
                    new PyString(@"C:\Users\denis.akhiyarov\source\repos\memoryleakpykw\pyinclr"));
                pymodule = PythonEngine.ImportModule("pyinclr");
                testmethod = pymodule.GetAttr("testmethod");
            }

            PythonEngine.ReleaseLock(pylock);

            Console.WriteLine("starting test for memory leak, press any key");
            Console.ReadKey();

            pylock = PythonEngine.AcquireLock();

            {
                for (int i=1; i<=1000000; i++)
                {
                    testmethod.Invoke();
                }
            }

            gc.InvokeMethod("collect"); 

            PythonEngine.ReleaseLock(pylock);
            

            GC.Collect();
            GC.WaitForPendingFinalizers(); // should no block
            GC.Collect();

            GC.WaitForPendingFinalizers(); // should no block

            PythonEngine.EndAllowThread(ts);

            Console.WriteLine("finished test for memory leak, press any key");
            Console.ReadKey();
        }

    }
}

@den-run-ai
Copy link
Contributor Author

@dmitriyse I'm not sure how is that supposed to help with the memory leak, but the leak is still there and not released from both GC. WaitForPendingFinalizers still blocks.

@den-run-ai
Copy link
Contributor Author

I got rid of this memory leak by using the following inconvenient disposal code:

PyObject res = testmethod.Invoke();
res.Dispose();

Not sure why res cannot be disposed by either GC?!

@den-run-ai
Copy link
Contributor Author

I still don't understand why managed GC from CLR is not calling the finalizer of PyObject for the results of calls into pythonnet:

        ~PyObject()
        {
            Dispose();
        }

What is holding the reference to these objects or why GC is not activated?

Thanks for the edit @dmitriyse, now when EndAllowThreads is after WaitForPendingFinalizers, then some of resources are released, but not all:

            PythonEngine.ReleaseLock(pylock);

            GC.Collect();
            GC.WaitForPendingFinalizers();
            GC.Collect();

            PythonEngine.EndAllowThreads(ts);

image

I read these Q&A and nothing looks wrong in PyObject:

https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/dispose-pattern

https://stackoverflow.com/questions/3038571/whats-the-purpose-of-gc-suppressfinalizethis-in-dispose-method

https://stackoverflow.com/questions/45036/will-the-garbage-collector-call-idisposable-dispose-for-me

https://stackoverflow.com/questions/3038571/whats-the-purpose-of-gc-suppressfinalizethis-in-dispose-method

https://stackoverflow.com/questions/538060/proper-use-of-the-idisposable-interface

https://stackoverflow.com/questions/898828/finalize-dispose-pattern-in-c-sharp

@dmitriyse
Copy link
Contributor

dmitriyse commented Sep 12, 2017

I tested this code in the branch #532.
And no any memory leaks occurred.
image
This image was captured after some minutes after start. So memory consumption is totally leak-free.

When I debugged this branch I faced with unpredictable C# GC behavior, after some amount of valid "Dispose" calls, GC does not generate enough finalizer calls. Looks like some unmanaged actions are not allowed inside GC thread.

@den-run-ai
Copy link
Contributor Author

@dmitriyse I tested your branch for finalizer with .NET Framework 4.6.1 and it looks great!

image

@den-run-ai
Copy link
Contributor Author

Interestingly .NET 4.0 garbage collector behaves much nicer:

image

@den-run-ai
Copy link
Contributor Author

Here is the testing for .NET Core 2.0, also looks very solid!

image

@den-run-ai
Copy link
Contributor Author

den-run-ai commented Sep 15, 2017

@dmitriyse here is another leak, even with your valid finalizer. This is when raising exception in Python and catching it in C#:

def testmethod(number=3, astring='abc'):
    raise ValueError("testing for memory leak")
    return astring * int(number)
using System;
using System.ComponentModel;
using Python.Runtime;

namespace memoryleakpykw
{
    class Program
    {
        static void Main(string[] args)
        {
            dynamic pymodule;
            PyObject gc;
            dynamic testmethod;
            PythonEngine.Initialize();
            var ts = PythonEngine.BeginAllowThreads();
            IntPtr pylock = PythonEngine.AcquireLock();
            {
                PyObject sys = PythonEngine.ImportModule("sys");// Py.Import("sys");
                gc = PythonEngine.ImportModule("gc");// Py.Import("sys");
                sys.GetAttr("path").InvokeMethod("append",
                    new PyString(@"C:\Users\denis.akhiyarov\source\repos\memoryleakpykw\pyinclr"));
                pymodule = PythonEngine.ImportModule("pyinclr");
                testmethod = pymodule.testmethod; //.GetAttr("testmethod");
            }
            PythonEngine.ReleaseLock(pylock);
            Console.WriteLine("starting test for memory leak, press any key");
            Console.ReadKey();
            double floatarg1 = 5.1f;
            dynamic res = null;
            {
                for (int i = 1; i <= 1000000; i++)
                {
                    using (Py.GIL())
                    {
                        try { 
                            res = testmethod(Py.kw("number", floatarg1), Py.kw("astring", "bbc"));
                        }
                        catch (Exception e)
                        {
                            if (i % 1000 == 0)
                            {
                                Console.WriteLine(e.Message);
                            }
                        }
                    }
                }
            }
            Console.WriteLine(res);           
            PythonEngine.EndAllowThreads(ts);
            Console.WriteLine("finished test for memory leak, press any key");
            Console.ReadKey();
        }
    }
}

@dmitriyse
Copy link
Contributor

dmitriyse commented Dec 12, 2017

Memory leak example with an exception is not so good. I tried the same example but without exception raising and it still produces memory leak (iterations count should be increased).

@dmitriyse
Copy link
Contributor

dmitriyse commented Dec 12, 2017

I fixed exception related memory leak. See latest update to PR #532

@dmitriyse
Copy link
Contributor

We needs to close this issue after PR #532 merge to master

@Zamgunner
Copy link

guys, any update on merging this fix to mainline?

@filmor filmor closed this as completed Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants