pythonnet · filmor · Jul 23, 2018 · Jul 20, 2018 · Jul 20, 2018 · Jul 20, 2018
diff --git a/AUTHORS.md b/AUTHORS.md
@@ -42,6 +42,7 @@
 -   Ville M. Vainio ([@vivainio](https://github.com/vivainio))
 -   Virgil Dupras ([@hsoft](https://github.com/hsoft))
 -   Wenguang Yang ([@yagweb](https://github.com/yagweb))
+-   William Sardar ([@williamsardar])(https://github.com/williamsardar)
 -   Xavier Dupré ([@sdpython](https://github.com/sdpython))
 -   Zane Purvis ([@zanedp](https://github.com/zanedp))
 -   ([@bltribble](https://github.com/bltribble))

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -35,6 +35,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
 -   Fixed `LockRecursionException` when loading assemblies ([#627][i627])
 -   Fixed errors breaking .NET Remoting on method invoke ([#276][i276])
 -   Fixed PyObject.GetHashCode ([#676][i676])
+-   Fix memory leaks due to spurious handle incrementation ([#691][i691])
 
 
 ## [2.3.0][] - 2017-03-11

diff --git a/requirements.txt b/requirements.txt
@@ -1,6 +1,7 @@
 # Requirements for both Travis and AppVeyor
 pytest==3.2.5
 coverage
+psutil
 
 # Coverage upload
 codecov

diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs
@@ -75,7 +75,7 @@ public static IntPtr tp_descr_get(IntPtr op, IntPtr instance, IntPtr owner)
                     return Exceptions.RaiseTypeError("How in the world could that happen!");
                 }
             }*/
-            Runtime.XIncref(self.pyHandle); // Decref'd by the interpreter.
+            Runtime.XIncref(self.pyHandle);
             return self.pyHandle;
         }
 
@@ -105,8 +105,6 @@ public static IntPtr mp_subscript(IntPtr op, IntPtr key)
             }
             var boundCtor = new BoundContructor(self.type, self.pyTypeHndl, self.ctorBinder, ci);
 
-            /* Since nothing is cached, do we need the increment???
-            Runtime.XIncref(boundCtor.pyHandle);  // Decref'd by the interpreter??? */
             return boundCtor.pyHandle;
         }
 

diff --git a/src/runtime/methodbinding.cs b/src/runtime/methodbinding.cs
@@ -56,7 +56,6 @@ public static IntPtr mp_subscript(IntPtr tp, IntPtr idx)
             }
 
             var mb = new MethodBinding(self.m, self.target) { info = mi };
-            Runtime.XIncref(mb.pyHandle);
             return mb.pyHandle;
         }
 
@@ -85,7 +84,6 @@ public static IntPtr tp_getattro(IntPtr ob, IntPtr key)
                 case "__overloads__":
                 case "Overloads":
                     var om = new OverloadMapper(self.m, self.target);
-                    Runtime.XIncref(om.pyHandle);
                     return om.pyHandle;
                 default:
                     return Runtime.PyObject_GenericGetAttr(ob, key);

diff --git a/src/runtime/overload.cs b/src/runtime/overload.cs
@@ -44,7 +44,6 @@ public static IntPtr mp_subscript(IntPtr tp, IntPtr idx)
             }
 
             var mb = new MethodBinding(self.m, self.target) { info = mi };
-            Runtime.XIncref(mb.pyHandle);
             return mb.pyHandle;
         }
 

diff --git a/src/testing/methodtest.cs b/src/testing/methodtest.cs
@@ -666,3 +666,23 @@ public string PublicMethod(string echo)
         }
     }
 }
+
+namespace PlainOldNamespace
+{
+    public class PlainOldClass
+    {
+        public PlainOldClass() { }
+
+        public PlainOldClass(int param) { }
+
+        private readonly byte[] payload = new byte[(int)Math.Pow(2, 20)]; //1 MB
+
+        public void NonGenericMethod() { }
+
+        public void GenericMethod<T>() { }
+
+        public void OverloadedMethod() { }
+
+        public void OverloadedMethod(int param) { }
+    }
+}
diff --git a/src/tests/test_method.py b/src/tests/test_method.py
@@ -832,3 +832,137 @@ def test_case_sensitive():
 
     with pytest.raises(AttributeError):
         MethodTest.casesensitive()
+
+def test_getting_generic_method_binding_does_not_leak_ref_count():
+    """Test that managed object is freed after calling generic method. Issue #691"""
+
+    from PlainOldNamespace import PlainOldClass
+
+    import sys
+
+    refCount = sys.getrefcount(PlainOldClass().GenericMethod[str])
+    assert refCount == 1
+
+def test_getting_generic_method_binding_does_not_leak_memory():
+    """Test that managed object is freed after calling generic method. Issue #691"""
+
+    from PlainOldNamespace import PlainOldClass
+
+    import psutil, os, gc, clr
+
+    process = psutil.Process(os.getpid())
+    processBytesBeforeCall = process.memory_info().rss
+    print("\n\nMemory consumption (bytes) at start of test: " + str(processBytesBeforeCall))
+
+    iterations = 500
+    for i in range(iterations):
+        PlainOldClass().GenericMethod[str]
+
+    gc.collect()
+    clr.System.GC.Collect()
+
+    processBytesAfterCall = process.memory_info().rss
+    print("Memory consumption (bytes) at end of test: " + str(processBytesAfterCall))
+    processBytesDelta = processBytesAfterCall - processBytesBeforeCall
+    print("Memory delta: " + str(processBytesDelta))
+
+    bytesAllocatedPerIteration = pow(2, 20)  # 1MB
+    bytesLeakedPerIteration = processBytesDelta / iterations
+
+    # Allow 50% threshold - this shows the original issue is fixed, which leaks the full allocated bytes per iteration
+    failThresholdBytesLeakedPerIteration = bytesAllocatedPerIteration / 2
+
+    assert bytesLeakedPerIteration < failThresholdBytesLeakedPerIteration
+
+def test_getting_overloaded_method_binding_does_not_leak_ref_count():
+    """Test that managed object is freed after calling overloaded method. Issue #691"""
+
+    from PlainOldNamespace import PlainOldClass
+
+    import sys
+
+    refCount = sys.getrefcount(PlainOldClass().OverloadedMethod.Overloads[int])
+    assert refCount == 1
+
+def test_getting_overloaded_method_binding_does_not_leak_memory():
+    """Test that managed object is freed after calling overloaded method. Issue #691"""
+
+    from PlainOldNamespace import PlainOldClass
+
+    import psutil, os, gc, clr
+
+    process = psutil.Process(os.getpid())
+    processBytesBeforeCall = process.memory_info().rss
+    print("\n\nMemory consumption (bytes) at start of test: " + str(processBytesBeforeCall))
+
+    iterations = 500
+    for i in range(iterations):
+        PlainOldClass().OverloadedMethod.Overloads[int]
+
+    gc.collect()
+    clr.System.GC.Collect()
+
+    processBytesAfterCall = process.memory_info().rss
+    print("Memory consumption (bytes) at end of test: " + str(processBytesAfterCall))
+    processBytesDelta = processBytesAfterCall - processBytesBeforeCall
+    print("Memory delta: " + str(processBytesDelta))
+
+    bytesAllocatedPerIteration = pow(2, 20)  # 1MB
+    bytesLeakedPerIteration = processBytesDelta / iterations
+
+    # Allow 50% threshold - this shows the original issue is fixed, which leaks the full allocated bytes per iteration
+    failThresholdBytesLeakedPerIteration = bytesAllocatedPerIteration / 2
+
+    assert bytesLeakedPerIteration < failThresholdBytesLeakedPerIteration
+
+def test_getting_method_overloads_binding_does_not_leak_ref_count():
+    """Test that managed object is freed after calling overloaded method. Issue #691"""
+
+    from PlainOldNamespace import PlainOldClass
+
+    import sys
+
+    refCount = sys.getrefcount(PlainOldClass().OverloadedMethod.Overloads)
+    assert refCount == 1
+
+def test_getting_method_overloads_binding_does_not_leak_memory():
+    """Test that managed object is freed after calling overloaded method. Issue #691"""
+
+    from PlainOldNamespace import PlainOldClass
+
+    import psutil, os, gc, clr
+
+    process = psutil.Process(os.getpid())
+    processBytesBeforeCall = process.memory_info().rss
+    print("\n\nMemory consumption (bytes) at start of test: " + str(processBytesBeforeCall))
+
+    iterations = 500
+    for i in range(iterations):
+        PlainOldClass().OverloadedMethod.Overloads
+
+    gc.collect()
+    clr.System.GC.Collect()
+
+    processBytesAfterCall = process.memory_info().rss
+    print("Memory consumption (bytes) at end of test: " + str(processBytesAfterCall))
+    processBytesDelta = processBytesAfterCall - processBytesBeforeCall
+    print("Memory delta: " + str(processBytesDelta))
+
+    bytesAllocatedPerIteration = pow(2, 20)  # 1MB
+    bytesLeakedPerIteration = processBytesDelta / iterations
+
+    # Allow 50% threshold - this shows the original issue is fixed, which leaks the full allocated bytes per iteration
+    failThresholdBytesLeakedPerIteration = bytesAllocatedPerIteration / 2
+
+    assert bytesLeakedPerIteration < failThresholdBytesLeakedPerIteration
+
+def test_getting_overloaded_constructor_binding_does_not_leak_ref_count():
+    """Test that managed object is freed after calling overloaded constructor, constructorbinding.cs mp_subscript. Issue #691"""
+
+    from PlainOldNamespace import PlainOldClass
+
+    import sys
+
+    # simple test
+    refCount = sys.getrefcount(PlainOldClass.Overloads[int])
+    assert refCount == 1
-Original file line number
+Diff line change
@@ Expand Up / @@ -44,7 +44,6 @@ public static IntPtr mp_subscript(IntPtr tp, IntPtr idx) @@
                 }
                 var mb = new MethodBinding(self.m, self.target) { info = mi };
-                Runtime.XIncref(mb.pyHandle);
                 return mb.pyHandle;
             }
@@ Expand Down @@