Another shot at #495 (#503)

rickardraysearch · filmor · commit a5f5dc2c2c1b · 2017-06-26T12:29:00.000+02:00
* Add tests of subclassing with __namespace__

* Remove __init__ call from ClassDerived.InvokeCtor

* Trying out __init__

* Cleanup

* Add tests constructing python type from CLR and calling __init__

* Revert borked changelog update

* Don't leak init reference

* Rename tests

* Remove unused internal Runtime.GetBoundArgTuple

* Reenable skipped tests in test_subclass.py
diff --git 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
diff --git 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;
@@ -808,7 +808,6 @@ public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, objec
                 obj,
                 args);
 
-            var disposeList = new List<PyObject>();
             CLRObject self = null;
             IntPtr gs = Runtime.PyGILState_Ensure();
             try
@@ -821,42 +820,9 @@ public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, objec
                 // object to be collected.
                 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
             {
-                foreach (PyObject x in disposeList)
-                {
-                    x?.Dispose();
-                }
-
                 // Decrement the python object's reference count.
                 // This doesn't actually destroy the object, it just sets the reference to this object
                 // to be a weak reference and it will be destroyed when the C# object is destroyed.
diff --git 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
@@ -157,23 +157,13 @@ public static IntPtr tp_call(IntPtr tp, IntPtr args, IntPtr kw)
                 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__);
+            var init = Runtime.PyObject_GetAttrString(obj, "__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);
+                IntPtr result = Runtime.PyObject_Call(init, args, kw);
+                Runtime.XDecref(init);
 
                 if (result == IntPtr.Zero)
                 {
diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs
@@ -378,29 +378,6 @@ internal static void CheckExceptionOccurred()
             }
         }
 
-        internal static IntPtr GetBoundArgTuple(IntPtr obj, IntPtr args)
-        {
-            if (PyObject_TYPE(args) != PyTupleType)
-            {
-                Exceptions.SetError(Exceptions.TypeError, "tuple expected");
-                return IntPtr.Zero;
-            }
-            int size = PyTuple_Size(args);
-            IntPtr items = PyTuple_New(size + 1);
-            PyTuple_SetItem(items, 0, obj);
-            XIncref(obj);
-
-            for (var i = 0; i < size; i++)
-            {
-                IntPtr item = PyTuple_GetItem(args, i);
-                XIncref(item);
-                PyTuple_SetItem(items, i + 1, item);
-            }
-
-            return items;
-        }
-
-
         internal static IntPtr ExtendTuple(IntPtr t, params IntPtr[] args)
         {
             int size = PyTuple_Size(t);
diff --git a/src/tests/test_subclass.py b/src/tests/test_subclass.py
@@ -15,12 +15,12 @@
 from ._compat import range
 
 
-def interface_test_class_fixture():
+def interface_test_class_fixture(subnamespace):
     """Delay creation of class until test starts."""
 
     class InterfaceTestClass(IInterfaceTest):
         """class that implements the test interface"""
-        __namespace__ = "Python.Test"
+        __namespace__ = "Python.Test." + subnamespace
 
         def foo(self):
             return "InterfaceTestClass"
@@ -31,12 +31,12 @@ def bar(self, x, i):
     return InterfaceTestClass
 
 
-def derived_class_fixture():
+def derived_class_fixture(subnamespace):
     """Delay creation of class until test starts."""
 
     class DerivedClass(SubClassTest):
         """class that derives from a class deriving from IInterfaceTest"""
-        __namespace__ = "Python.Test"
+        __namespace__ = "Python.Test." + subnamespace
 
         def foo(self):
             return "DerivedClass"
@@ -60,12 +60,12 @@ def return_list(self):
     return DerivedClass
 
 
-def derived_event_test_class_fixture():
+def derived_event_test_class_fixture(subnamespace):
     """Delay creation of class until test starts."""
 
     class DerivedEventTest(IInterfaceTest):
         """class that implements IInterfaceTest.TestEvent"""
-        __namespace__ = "Python.Test"
+        __namespace__ = "Python.Test." + subnamespace
 
         def __init__(self):
             self.event_handlers = []
@@ -99,7 +99,7 @@ def test_base_class():
 
 def test_interface():
     """Test python classes can derive from C# interfaces"""
-    InterfaceTestClass = interface_test_class_fixture()
+    InterfaceTestClass = interface_test_class_fixture(test_interface.__name__)
     ob = InterfaceTestClass()
     assert ob.foo() == "InterfaceTestClass"
     assert FunctionsTest.test_foo(ob) == "InterfaceTestClass"
@@ -112,7 +112,7 @@ def test_interface():
 
 def test_derived_class():
     """Test python class derived from managed type"""
-    DerivedClass = derived_class_fixture()
+    DerivedClass = derived_class_fixture(test_derived_class.__name__)
     ob = DerivedClass()
     assert ob.foo() == "DerivedClass"
     assert ob.base_foo() == "foo"
@@ -128,10 +128,9 @@ def test_derived_class():
     assert id(x) == id(ob)
 
 
-@pytest.mark.skip(reason="FIXME: test randomly pass/fails")
 def test_create_instance():
     """Test derived instances can be created from managed code"""
-    DerivedClass = derived_class_fixture()
+    DerivedClass = derived_class_fixture(test_create_instance.__name__)
     ob = FunctionsTest.create_instance(DerivedClass)
     assert ob.foo() == "DerivedClass"
     assert FunctionsTest.test_foo(ob) == "DerivedClass"
@@ -142,7 +141,7 @@ def test_create_instance():
     x = FunctionsTest.pass_through(ob)
     assert id(x) == id(ob)
 
-    InterfaceTestClass = interface_test_class_fixture()
+    InterfaceTestClass = interface_test_class_fixture(test_create_instance.__name__)
     ob2 = FunctionsTest.create_instance(InterfaceTestClass)
     assert ob2.foo() == "InterfaceTestClass"
     assert FunctionsTest.test_foo(ob2) == "InterfaceTestClass"
@@ -153,7 +152,6 @@ def test_create_instance():
     assert id(y) == id(ob2)
 
 
-@pytest.mark.skip(reason="FIXME: test randomly pass/fails")
 def test_events():
     class EventHandler(object):
         def handler(self, x, args):
@@ -166,12 +164,12 @@ def handler(self, x, args):
     assert FunctionsTest.test_event(x, 1) == 1
     assert event_handler.value == 1
 
-    InterfaceTestClass = interface_test_class_fixture()
+    InterfaceTestClass = interface_test_class_fixture(test_events.__name__)
     i = InterfaceTestClass()
     with pytest.raises(System.NotImplementedException):
         FunctionsTest.test_event(i, 2)
 
-    DerivedEventTest = derived_event_test_class_fixture()
+    DerivedEventTest = derived_event_test_class_fixture(test_events.__name__)
     d = DerivedEventTest()
     d.add_TestEvent(event_handler.handler)
     assert FunctionsTest.test_event(d, 3) == 3
@@ -190,3 +188,59 @@ def test_isinstance_check():
     for x in b:
         assert isinstance(x, System.Object)
         assert isinstance(x, System.String)
+
+def test_namespace_and_init():
+    calls = []
+    class TestX(System.Object):
+        __namespace__ = "test_clr_subclass_with_init_args"
+        def __init__(self, *args, **kwargs):
+            calls.append((args, kwargs))
+    t = TestX(1,2,3,foo="bar")
+    assert len(calls) == 1
+    assert calls[0][0] == (1,2,3)
+    assert calls[0][1] == {"foo":"bar"}
+
+def test_namespace_and_argless_init():
+    calls = []
+    class TestX(System.Object):
+        __namespace__ = "test_clr_subclass_without_init_args"
+        def __init__(self):
+            calls.append(True)
+    t = TestX()
+    assert len(calls) == 1
+    assert calls[0] == True
+
+
+def test_namespace_and_no_init():
+    class TestX(System.Object):
+        __namespace__ = "test_clr_subclass_without_init"
+        q = 1
+    t = TestX()
+    assert t.q == 1
+
+def test_construction_from_clr():
+    import clr
+    calls = []
+    class TestX(System.Object):
+        __namespace__ = "test_clr_subclass_init_from_clr"
+        @clr.clrmethod(None, [int, str])
+        def __init__(self, i, s):
+            calls.append((i, s))
+
+    # Construct a TestX from Python
+    t = TestX(1, "foo")
+    assert len(calls) == 1
+    assert calls[0][0] == 1
+    assert calls[0][1] == "foo"
+
+    # Reset calls and construct a TestX from CLR
+    calls = []
+    tp = t.GetType()
+    t2 = tp.GetConstructors()[0].Invoke(None)
+    assert len(calls) == 0
+
+    # The object has only been constructed, now it needs to be initialized as well
+    tp.GetMethod("__init__").Invoke(t2, [1, "foo"])
+    assert len(calls) == 1
+    assert calls[0][0] == 1
+    assert calls[0][1] == "foo"
