From 9f64f5637e433b5d946051e2f8780c48dbbf36bf Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Fri, 16 Jun 2017 19:54:11 +0200 Subject: [PATCH 1/5] Drop tp_call implementation from metatype. This seems to be a patch for broken behaviour in Python. Removing it should solve #495 (`__init__` called twice on construction). --- src/runtime/metatype.cs | 53 +------------------------------------- src/tests/test_subclass.py | 18 +++++++++++++ 2 files changed, 19 insertions(+), 52 deletions(-) diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs index bfb71e26d..03cdfd992 100644 --- a/src/runtime/metatype.cs +++ b/src/runtime/metatype.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Runtime.InteropServices; namespace Python.Runtime @@ -137,57 +137,6 @@ public static void tp_free(IntPtr tp) Runtime.PyObject_GC_Del(tp); } - - /// - /// Metatype __call__ implementation. This is needed to ensure correct - /// initialization (__init__ support), because the tp_call we inherit - /// from PyType_Type won't call __init__ for metatypes it doesn't know. - /// - public static IntPtr tp_call(IntPtr tp, IntPtr args, IntPtr kw) - { - IntPtr func = Marshal.ReadIntPtr(tp, TypeOffset.tp_new); - if (func == IntPtr.Zero) - { - return Exceptions.RaiseTypeError("invalid object"); - } - - IntPtr obj = NativeCall.Call_3(func, tp, args, kw); - if (obj == IntPtr.Zero) - { - return IntPtr.Zero; - } - - IntPtr py__init__ = Runtime.PyString_FromString("__init__"); - IntPtr type = Runtime.PyObject_TYPE(obj); - IntPtr init = Runtime._PyType_Lookup(type, py__init__); - Runtime.XDecref(py__init__); - Runtime.PyErr_Clear(); - - if (init != IntPtr.Zero) - { - IntPtr bound = Runtime.GetBoundArgTuple(obj, args); - if (bound == IntPtr.Zero) - { - Runtime.XDecref(obj); - return IntPtr.Zero; - } - - IntPtr result = Runtime.PyObject_Call(init, bound, kw); - Runtime.XDecref(bound); - - if (result == IntPtr.Zero) - { - Runtime.XDecref(obj); - return IntPtr.Zero; - } - - Runtime.XDecref(result); - } - - return obj; - } - - /// /// Type __setattr__ implementation for reflected types. Note that this /// is slightly different than the standard setattr implementation for diff --git a/src/tests/test_subclass.py b/src/tests/test_subclass.py index 739c24c07..9dab42d3c 100644 --- a/src/tests/test_subclass.py +++ b/src/tests/test_subclass.py @@ -190,3 +190,21 @@ def test_isinstance_check(): for x in b: assert isinstance(x, System.Object) assert isinstance(x, System.String) + + +_derived_cons_calls = 0 + +def test_derived_constructor_only_called_once(): + global _derived_cons_calls + _derived_cons_calls = 0 + + class X(System.Object): + __namespace__ = "PyTest" + + def __init__(self): + global _derived_cons_calls + _derived_cons_calls += 1 + + x = X() + + assert _derived_cons_calls == 1 From 0f7876f3cebba9f52d42c15f94b668b5acbf160f Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Fri, 16 Jun 2017 20:15:40 +0200 Subject: [PATCH 2/5] Add changelog entry. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38fa56a62..5ae62d692 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. attribute (#481) - Fixed conversion of 'float' and 'double' values (#486) - Fixed 'clrmethod' for python 2 (#492) +- Fixed double calling of constructor when deriving from .NET class (#495) ## [2.3.0][] - 2017-03-11 From 0dda85ac5ae73ac4f9987d30bba55af84f426224 Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Fri, 16 Jun 2017 21:21:28 +0200 Subject: [PATCH 3/5] Improve test. --- src/tests/test_subclass.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/tests/test_subclass.py b/src/tests/test_subclass.py index 9dab42d3c..40b8a65f5 100644 --- a/src/tests/test_subclass.py +++ b/src/tests/test_subclass.py @@ -192,19 +192,15 @@ def test_isinstance_check(): assert isinstance(x, System.String) -_derived_cons_calls = 0 - def test_derived_constructor_only_called_once(): - global _derived_cons_calls - _derived_cons_calls = 0 + calls = [] class X(System.Object): __namespace__ = "PyTest" - def __init__(self): - global _derived_cons_calls - _derived_cons_calls += 1 + def __init__(self, *args): + calls.append(args) - x = X() + x = X(1, 2) - assert _derived_cons_calls == 1 + assert calls == [(1, 2)] From 5cf42daa7f6bcfed1606b2648b80714dd5ac776f Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Mon, 19 Jun 2017 12:46:24 +0200 Subject: [PATCH 4/5] Move __init__ call in ClassDerived to tp_init. --- src/runtime/classderived.cs | 57 +++++++++++++++++++------------------ src/runtime/metatype.cs | 51 +++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 27 deletions(-) diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs index c180f9acc..8b7563621 100644 --- a/src/runtime/classderived.cs +++ b/src/runtime/classderived.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -55,6 +55,33 @@ internal ClassDerivedObject(Type tp) : base(tp) return Converter.ToPython(obj, cls.GetType()); } + public new static int tp_init(IntPtr obj, IntPtr args, IntPtr kw) + { + Runtime.XIncref(obj); + Runtime.XIncref(Runtime.PyNone); + using (var pyself = new PyObject(obj)) + using (var pynone = new PyObject(Runtime.PyNone)) + using (var init = pyself.GetAttr("__init__", pynone)) + { + if (init.Handle != Runtime.PyNone) + { + // if __init__ hasn't been overridden then it will be a managed object + if (GetManagedObject(init.Handle) == null) + { + Runtime.XIncref(args); + Runtime.XIncref(kw); + using (var args_ = new PyTuple(args)) + using (var kw_ = new PyDict(kw)) + using (var res = init.Invoke(args_, kw_)) + { + } + } + } + } + + return 0; + } + public new static void tp_dealloc(IntPtr ob) { var self = (CLRObject)GetManagedObject(ob); @@ -801,6 +828,8 @@ public static void InvokeSetProperty(IPythonDerivedType obj, string propertyN public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, object[] args) { + Console.Error.WriteLine($"{origCtorName}: {string.Join(", ", args)}"); + // call the base constructor obj.GetType().InvokeMember(origCtorName, BindingFlags.InvokeMethod, @@ -822,33 +851,7 @@ public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, objec FieldInfo fi = obj.GetType().GetField("__pyobj__"); fi.SetValue(obj, self); - Runtime.XIncref(self.pyHandle); - var pyself = new PyObject(self.pyHandle); - disposeList.Add(pyself); - - Runtime.XIncref(Runtime.PyNone); - var pynone = new PyObject(Runtime.PyNone); - disposeList.Add(pynone); - // call __init__ - PyObject init = pyself.GetAttr("__init__", pynone); - disposeList.Add(init); - if (init.Handle != Runtime.PyNone) - { - // if __init__ hasn't been overridden then it will be a managed object - ManagedType managedMethod = ManagedType.GetManagedObject(init.Handle); - if (null == managedMethod) - { - var pyargs = new PyObject[args.Length]; - for (var i = 0; i < args.Length; ++i) - { - pyargs[i] = new PyObject(Converter.ToPython(args[i], args[i]?.GetType())); - disposeList.Add(pyargs[i]); - } - - disposeList.Add(init.Invoke(pyargs)); - } - } } finally { diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs index 03cdfd992..bd0d8c3e8 100644 --- a/src/runtime/metatype.cs +++ b/src/runtime/metatype.cs @@ -125,6 +125,57 @@ public static IntPtr tp_new(IntPtr tp, IntPtr args, IntPtr kw) } + /// + /// Metatype __call__ implementation. This is needed to ensure correct + /// initialization (__init__ support), because the tp_call we inherit + /// from PyType_Type won't call __init__ for metatypes it doesn't know. + /// + public static IntPtr tp_call(IntPtr tp, IntPtr args, IntPtr kw) + { + IntPtr func = Marshal.ReadIntPtr(tp, TypeOffset.tp_new); + if (func == IntPtr.Zero) + { + return Exceptions.RaiseTypeError("invalid object"); + } + + IntPtr obj = NativeCall.Call_3(func, tp, args, kw); + if (obj == IntPtr.Zero) + { + return IntPtr.Zero; + } + + IntPtr py__init__ = Runtime.PyString_FromString("__init__"); + IntPtr type = Runtime.PyObject_TYPE(obj); + IntPtr init = Runtime._PyType_Lookup(type, py__init__); + Runtime.XDecref(py__init__); + Runtime.PyErr_Clear(); + + if (init != IntPtr.Zero) + { + IntPtr bound = Runtime.GetBoundArgTuple(obj, args); + if (bound == IntPtr.Zero) + { + Runtime.XDecref(obj); + return IntPtr.Zero; + } + + IntPtr result = Runtime.PyObject_Call(init, bound, kw); + Runtime.XDecref(bound); + + if (result == IntPtr.Zero) + { + Runtime.XDecref(obj); + return IntPtr.Zero; + } + + Runtime.XDecref(result); + } + + return obj; + } + + + public static IntPtr tp_alloc(IntPtr mt, int n) { IntPtr type = Runtime.PyType_GenericAlloc(mt, n); From 096cf7a29170639f539f156a6e826c3ec66fcf4d Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Mon, 19 Jun 2017 17:50:12 +0200 Subject: [PATCH 5/5] Remove debug logging. --- src/runtime/classderived.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs index 8b7563621..6a10a5add 100644 --- a/src/runtime/classderived.cs +++ b/src/runtime/classderived.cs @@ -828,8 +828,6 @@ public static void InvokeSetProperty(IPythonDerivedType obj, string propertyN public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, object[] args) { - Console.Error.WriteLine($"{origCtorName}: {string.Join(", ", args)}"); - // call the base constructor obj.GetType().InvokeMember(origCtorName, BindingFlags.InvokeMethod,