diff --git a/src/domain_tests/test_domain_reload.py b/src/domain_tests/test_domain_reload.py index a7cd2fa4d..b54f7c495 100644 --- a/src/domain_tests/test_domain_reload.py +++ b/src/domain_tests/test_domain_reload.py @@ -56,7 +56,6 @@ def test_property_visibility_change(): def test_class_visibility_change(): _run_test("class_visibility_change") -@pytest.mark.skip(reason='FIXME: Domain reload fails when Python points to a .NET object which points back to Python objects') def test_method_parameters_change(): _run_test("method_parameters_change") diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index 8b96a96da..60a66829a 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -517,5 +517,11 @@ public static int mp_ass_subscript(IntPtr ob, IntPtr idx, IntPtr v) return 0; } + + public override string ToString() + { + return $""; + } + } } diff --git a/src/runtime/clrobject.cs b/src/runtime/clrobject.cs index 0a352b381..1a1a6c987 100644 --- a/src/runtime/clrobject.cs +++ b/src/runtime/clrobject.cs @@ -103,5 +103,10 @@ protected override void OnLoad(InterDomainContext context) GCHandle gc = AllocGCHandle(TrackTypes.Wrapper); Marshal.WriteIntPtr(pyHandle, ObjectOffset.magic(tpHandle), (IntPtr)gc); } + + public override string ToString() + { + return $""; + } } } diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index da8653853..3269b3883 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -85,6 +85,29 @@ internal static Exception ToException(IntPtr ob) } return Runtime.PyUnicode_FromString(message); } + + static void ClearOffsetHelper(IntPtr ob, int offset) + { + var field = Marshal.ReadIntPtr(ob, offset); + if (field != IntPtr.Zero) + { + Marshal.WriteIntPtr(ob, offset, IntPtr.Zero); + Runtime.XDecref(field); + } + } + + // As seen in exceptions.c, every derived type must also clean the base. + // TODO: once the call tp_traverse and tp_clear on the instance and not + // the type, implement those. + public new static void tp_dealloc(IntPtr self) + { + ClearOffsetHelper(self, ExceptionOffset.dict); + ClearOffsetHelper(self, ExceptionOffset.args); + ClearOffsetHelper(self, ExceptionOffset.traceback); + ClearOffsetHelper(self, ExceptionOffset.context); + ClearOffsetHelper(self, ExceptionOffset.cause); + ClassBase.tp_dealloc(self); + } } /// @@ -183,7 +206,7 @@ internal static void SetArgsAndCause(IntPtr ob) { // Note: For an AggregateException, InnerException is only the first of the InnerExceptions. IntPtr cause = CLRObject.GetInstHandle(e.InnerException); - Marshal.WriteIntPtr(ob, ExceptionOffset.cause, cause); + Runtime.PyException_SetCause(ob, cause); } } @@ -279,10 +302,7 @@ public static void SetError(Exception e) var pe = e as PythonException; if (pe != null) { - Runtime.XIncref(pe.PyType); - Runtime.XIncref(pe.PyValue); - Runtime.XIncref(pe.PyTB); - Runtime.PyErr_Restore(pe.PyType, pe.PyValue, pe.PyTB); + pe.Restore(); return; } @@ -399,6 +419,7 @@ internal static IntPtr RaiseTypeError(string message) if (previousException != null) { SetCause(previousException); + previousException.Dispose(); } return IntPtr.Zero; } diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 13b45df51..2fd34a901 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -368,6 +368,7 @@ public static IntPtr __import__(IntPtr self, IntPtr argsRaw, IntPtr kw) } Runtime.XIncref(mod.pyHandle); + originalException.Dispose(); return mod.pyHandle; } } diff --git a/src/testing/exceptiontest.cs b/src/testing/exceptiontest.cs index 45acaa2cc..ce82f8817 100644 --- a/src/testing/exceptiontest.cs +++ b/src/testing/exceptiontest.cs @@ -1,3 +1,4 @@ +using Python.Runtime; using System; using System.Collections; using System.Collections.Generic; @@ -81,6 +82,38 @@ public static void ThrowChainedExceptions() throw new Exception("Outer exception", exc2); } } + public static IntPtr DoThrowSimple() + { + using (Py.GIL()) + { + // Set a python TypeError. + Exceptions.SetError(Exceptions.TypeError, "type error, The first."); + var pyerr = new PythonException(); + // PyErr_Fetch() it and set it as the cause of an ArgumentException (and raise). + throw new ArgumentException("Bogus bad parameter", pyerr); + } + } + + public static void DoThrowWithInner() + { + using(Py.GIL()) + { + // Set a python TypeError. + Exceptions.SetError(Exceptions.TypeError, "type error, The first."); + var pyerr = new PythonException(); + // PyErr_Fetch() it and set it as the cause of an ArgumentException (and raise). + Exceptions.SetError(new ArgumentException("Bogus bad parameter", pyerr)); + // But we want Python error.. raise a TypeError and set the cause to the + // previous ArgumentError. + PythonException previousException = new PythonException(); + Exceptions.SetError(Exceptions.TypeError, "type error, The second."); + Exceptions.SetCause(previousException); + previousException.Dispose(); + // Then throw-raise the TypeError. + throw new PythonException(); + } + } + } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 7fafeebcb..066facbf8 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -9,6 +9,53 @@ import pickle +# begin code from https://utcc.utoronto.ca/~cks/space/blog/python/GetAllObjects +import gc +# Recursively expand slist's objects +# into olist, using seen to track +# already processed objects. + +def _getr(slist, olist, seen): + for e in slist: + if id(e) in seen: + continue + seen[id(e)] = None + olist.append(e) + tl = gc.get_referents(e) + if tl: + _getr(tl, olist, seen) + +# The public function. +def get_all_objects(): + gcl = gc.get_objects() + olist = [] + seen = {} + # Just in case: + seen[id(gcl)] = None + seen[id(olist)] = None + seen[id(seen)] = None + # _getr does the real work. + _getr(gcl, olist, seen) + return olist +# end code from https://utcc.utoronto.ca/~cks/space/blog/python/GetAllObjects + +def leak_check(func): + def do_leak_check(): + func() + gc.collect() + exc = {x for x in get_all_objects() if isinstance(x, Exception) and not isinstance(x, pytest.PytestDeprecationWarning)} + print(len(exc)) + if len(exc): + for x in exc: + print('-------') + print(repr(x)) + print(gc.get_referrers(x)) + print(len(gc.get_referrers(x))) + assert False + gc.collect() + return do_leak_check + + def test_unified_exception_semantics(): """Test unified exception semantics.""" e = System.Exception('Something bad happened') @@ -375,3 +422,31 @@ def test_iteration_innerexception(): # after exception is thrown iterator is no longer valid with pytest.raises(StopIteration): next(val) + +def leak_test(func): + def do_test_leak(): + # PyTest leaks things, gather the current state + orig_exc = {x for x in get_all_objects() if isinstance(x, Exception)} + func() + exc = {x for x in get_all_objects() if isinstance(x, Exception)} + possibly_leaked = exc - orig_exc + assert not possibly_leaked + + return do_test_leak + +@leak_test +def test_dont_leak_exceptions_simple(): + from Python.Test import ExceptionTest + + try: + ExceptionTest.DoThrowSimple() + except System.ArgumentException: + print('type error, as expected') + +@leak_test +def test_dont_leak_exceptions_inner(): + from Python.Test import ExceptionTest + try: + ExceptionTest.DoThrowWithInner() + except TypeError: + print('type error, as expected')