Skip to content

memory leak - simple function call in a loop #541

Closed

Description

@den-run-ai

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();
        }

    }
}

Activity

  1. den-run-ai commented on Sep 11, 2017

    @den-run-ai
    ContributorAuthor

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

  2. dmitriyse commented on Sep 11, 2017

    @dmitriyse
    Contributor

    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();
            }
    
        }
    }
  3. den-run-ai commented on Sep 11, 2017

    @den-run-ai
    ContributorAuthor

    @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.

  4. den-run-ai commented on Sep 11, 2017

    @den-run-ai
    ContributorAuthor

    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?!

  5. den-run-ai commented on Sep 12, 2017

    @den-run-ai
    ContributorAuthor

    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

  6. dmitriyse commented on Sep 12, 2017

    @dmitriyse
    Contributor

    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.

  7. den-run-ai commented on Sep 13, 2017

    @den-run-ai
    ContributorAuthor

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

    image

  8. den-run-ai commented on Sep 13, 2017

    @den-run-ai
    ContributorAuthor

    Interestingly .NET 4.0 garbage collector behaves much nicer:

    image

  9. den-run-ai commented on Sep 14, 2017

    @den-run-ai
    ContributorAuthor

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

    image

  10. den-run-ai commented on Sep 15, 2017

    @den-run-ai
    ContributorAuthor

    @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();
            }
        }
    }
  11. dmitriyse commented on Dec 12, 2017

    @dmitriyse
    Contributor

    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).

  12. dmitriyse commented on Dec 12, 2017

    @dmitriyse
    Contributor

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

  13. dmitriyse commented on Dec 12, 2017

    @dmitriyse
    Contributor

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

  14. Zamgunner commented on Nov 14, 2018

    @Zamgunner

    guys, any update on merging this fix to mainline?

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions