Skip to content
This repository was archived by the owner on Jul 22, 2023. It is now read-only.

Commit b2e1bb2

Browse files
williamsardarwilliamsardar
committed
Fix memory leaks and add unit tests
1 parent 611814c commit b2e1bb2

File tree

5 files changed

+154
-4
lines changed

5 files changed

+154
-4
lines changed

‎src/runtime/constructorbinding.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ public static IntPtr tp_descr_get(IntPtr op, IntPtr instance, IntPtr owner)
7575
return Exceptions.RaiseTypeError("How in the world could that happen!");
7676
}
7777
}*/
78-
Runtime.XIncref(self.pyHandle); // Decref'd by the interpreter.
7978
return self.pyHandle;
8079
}
8180

‎src/runtime/methodbinding.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ public static IntPtr mp_subscript(IntPtr tp, IntPtr idx)
5656
}
5757

5858
var mb = new MethodBinding(self.m, self.target) { info = mi };
59-
Runtime.XIncref(mb.pyHandle);
6059
return mb.pyHandle;
6160
}
6261

@@ -85,7 +84,6 @@ public static IntPtr tp_getattro(IntPtr ob, IntPtr key)
8584
case "__overloads__":
8685
case "Overloads":
8786
var om = new OverloadMapper(self.m, self.target);
88-
Runtime.XIncref(om.pyHandle);
8987
return om.pyHandle;
9088
default:
9189
return Runtime.PyObject_GenericGetAttr(ob, key);

‎src/runtime/overload.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ public static IntPtr mp_subscript(IntPtr tp, IntPtr idx)
4444
}
4545

4646
var mb = new MethodBinding(self.m, self.target) { info = mi };
47-
Runtime.XIncref(mb.pyHandle);
4847
return mb.pyHandle;
4948
}
5049

‎src/testing/methodtest.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,3 +666,23 @@ public string PublicMethod(string echo)
666666
}
667667
}
668668
}
669+
670+
namespace PlainOldNamespace
671+
{
672+
public class PlainOldClass
673+
{
674+
public PlainOldClass() { }
675+
676+
public PlainOldClass(int param) { }
677+
678+
private readonly byte[] payload = new byte[(int)Math.Pow(2, 20)]; //1 MB
679+
680+
public void NonGenericMethod() { }
681+
682+
public void GenericMethod<T>() { }
683+
684+
public void OverloadedMethod() { }
685+
686+
public void OverloadedMethod(int param) { }
687+
}
688+
}

‎src/tests/test_method.py‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,3 +832,137 @@ def test_case_sensitive():
832832

833833
with pytest.raises(AttributeError):
834834
MethodTest.casesensitive()
835+
836+
def test_getting_generic_method_binding_does_not_leak_ref_count():
837+
"""Test that managed object is freed after calling generic method. Issue #691"""
838+
839+
from PlainOldNamespace import PlainOldClass
840+
841+
import sys
842+
843+
refCount = sys.getrefcount(PlainOldClass().GenericMethod[str])
844+
assert refCount == 1
845+
846+
def test_getting_generic_method_binding_does_not_leak_memory():
847+
"""Test that managed object is freed after calling generic method. Issue #691"""
848+
849+
from PlainOldNamespace import PlainOldClass
850+
851+
import psutil, os, gc, clr
852+
853+
process = psutil.Process(os.getpid())
854+
processBytesBeforeCall = process.memory_info().private
855+
print("\n\nMemory consumption (bytes) at start of test: " + str(processBytesBeforeCall))
856+
857+
iterations = 500
858+
for i in range(iterations):
859+
PlainOldClass().GenericMethod[str]
860+
861+
gc.collect()
862+
clr.System.GC.Collect()
863+
864+
processBytesAfterCall = process.memory_info().private
865+
print("Memory consumption (bytes) at end of test: " + str(processBytesAfterCall))
866+
processBytesDelta = processBytesAfterCall - processBytesBeforeCall
867+
print("Memory delta: " + str(processBytesDelta))
868+
869+
bytesAllocatedPerIteration = pow(2, 20) # 1MB
870+
bytesLeakedPerIteration = processBytesDelta / iterations
871+
872+
# Allow 50% threshold - this shows the original issue is fixed, which leaks the full allocated bytes per iteration
873+
failThresholdBytesLeakedPerIteration = bytesAllocatedPerIteration / 2
874+
875+
assert bytesLeakedPerIteration < failThresholdBytesLeakedPerIteration
876+
877+
def test_getting_overloaded_method_binding_does_not_leak_ref_count():
878+
"""Test that managed object is freed after calling overloaded method. Issue #691"""
879+
880+
from PlainOldNamespace import PlainOldClass
881+
882+
import sys
883+
884+
refCount = sys.getrefcount(PlainOldClass().OverloadedMethod.Overloads[int])
885+
assert refCount == 1
886+
887+
def test_getting_overloaded_method_binding_does_not_leak_memory():
888+
"""Test that managed object is freed after calling overloaded method. Issue #691"""
889+
890+
from PlainOldNamespace import PlainOldClass
891+
892+
import psutil, os, gc, clr
893+
894+
process = psutil.Process(os.getpid())
895+
processBytesBeforeCall = process.memory_info().private
896+
print("\n\nMemory consumption (bytes) at start of test: " + str(processBytesBeforeCall))
897+
898+
iterations = 500
899+
for i in range(iterations):
900+
PlainOldClass().OverloadedMethod.Overloads[int]
901+
902+
gc.collect()
903+
clr.System.GC.Collect()
904+
905+
processBytesAfterCall = process.memory_info().private
906+
print("Memory consumption (bytes) at end of test: " + str(processBytesAfterCall))
907+
processBytesDelta = processBytesAfterCall - processBytesBeforeCall
908+
print("Memory delta: " + str(processBytesDelta))
909+
910+
bytesAllocatedPerIteration = pow(2, 20) # 1MB
911+
bytesLeakedPerIteration = processBytesDelta / iterations
912+
913+
# Allow 50% threshold - this shows the original issue is fixed, which leaks the full allocated bytes per iteration
914+
failThresholdBytesLeakedPerIteration = bytesAllocatedPerIteration / 2
915+
916+
assert bytesLeakedPerIteration < failThresholdBytesLeakedPerIteration
917+
918+
def test_getting_method_overloads_binding_does_not_leak_ref_count():
919+
"""Test that managed object is freed after calling overloaded method. Issue #691"""
920+
921+
from PlainOldNamespace import PlainOldClass
922+
923+
import sys
924+
925+
refCount = sys.getrefcount(PlainOldClass().OverloadedMethod.Overloads)
926+
assert refCount == 1
927+
928+
def test_getting_method_overloads_binding_does_not_leak_memory():
929+
"""Test that managed object is freed after calling overloaded method. Issue #691"""
930+
931+
from PlainOldNamespace import PlainOldClass
932+
933+
import psutil, os, gc, clr
934+
935+
process = psutil.Process(os.getpid())
936+
processBytesBeforeCall = process.memory_info().private
937+
print("\n\nMemory consumption (bytes) at start of test: " + str(processBytesBeforeCall))
938+
939+
iterations = 500
940+
for i in range(iterations):
941+
PlainOldClass().OverloadedMethod.Overloads
942+
943+
gc.collect()
944+
clr.System.GC.Collect()
945+
946+
processBytesAfterCall = process.memory_info().private
947+
print("Memory consumption (bytes) at end of test: " + str(processBytesAfterCall))
948+
processBytesDelta = processBytesAfterCall - processBytesBeforeCall
949+
print("Memory delta: " + str(processBytesDelta))
950+
951+
bytesAllocatedPerIteration = pow(2, 20) # 1MB
952+
bytesLeakedPerIteration = processBytesDelta / iterations
953+
954+
# Allow 50% threshold - this shows the original issue is fixed, which leaks the full allocated bytes per iteration
955+
failThresholdBytesLeakedPerIteration = bytesAllocatedPerIteration / 2
956+
957+
assert bytesLeakedPerIteration < failThresholdBytesLeakedPerIteration
958+
959+
def test_getting_overloaded_constructor_binding_does_not_leak_ref_count():
960+
"""Test that managed object is freed after calling overloaded constructor, constructorbinding.cs mp_subscript. Issue #691"""
961+
962+
from PlainOldNamespace import PlainOldClass
963+
964+
import sys
965+
966+
# simple test
967+
refCount = sys.getrefcount(PlainOldClass.Overloads[int])
968+
assert refCount == 1

0 commit comments

Comments
 (0)