Run a .NET constructor once when it is instantiated from Python

src/embed_tests/QCTest.cs

@@ -17,6 +17,16 @@ class QCTests
         private static dynamic withArgs_PythonSuperInitInt;
 
         private static dynamic pureCSharpConstruction;
+        private static dynamic pureCSharpDefaultConstruction;
+        private static dynamic pureCSharpOptionalArgsConstruction;
+        private static dynamic pureCSharpOptionalArgsDefaultConstruction;
+        private static dynamic pythonSuperInitDefaultAfterOtherConstruction;
+        private static dynamic reinitAfterConstruction;
+        private static dynamic pythonOptionalArgsSuperInitDefault;
+        private static dynamic pythonOptionalArgsSuperInitValue;
+        private static dynamic namespacedSuperInitNone;
+        private static dynamic namespacedSuperInitDefault;
+        private static dynamic pythonSelectedOverloadInit;
 
         private static dynamic containsTest;
         private static dynamic module;
@@ -25,6 +35,7 @@ from clr import AddReference
 AddReference(""System"")
 AddReference(""Python.EmbeddingTest"")
 from Python.EmbeddingTest import *
+import System
 class PythonModule(Algo):
     def TestA(self):
         try:
@@ -68,6 +79,45 @@ def jose(self):
 
 def PureCSharpConstruction():
     return SuperInit(1)
+
+def PureCSharpDefaultConstruction():
+    return SuperInit()
+
+def PureCSharpOptionalArgsConstruction():
+    return OptionalArgsInit(5)
+
+def PureCSharpOptionalArgsDefaultConstruction():
+    return OptionalArgsInit()
+
+class PythonSuperInitDefaultAfterOtherConstruction(SuperInit):
+    def __init__(self):
+        self.other = SuperInit()
+        super().__init__()
+
+class PythonOptionalArgsSuperInitDefault(OptionalArgsInit):
+    def __init__(self):
+        super().__init__()
+
+class PythonOptionalArgsSuperInitValue(OptionalArgsInit):
+    def __init__(self):
+        super().__init__(5)
+
+class PythonSelectedOverloadInit(SelectedOverloadInit):
+    def __init__(self):
+        super().__init__.__overloads__[System.String]()
+
+class NamespacedSuperInitNone(SuperInit):
+    __namespace__ = 'Python.EmbeddingTest.QCTests'
+
+class NamespacedSuperInitDefault(SuperInit):
+    __namespace__ = 'Python.EmbeddingTest.QCTests'
+    def __init__(self):
+        super().__init__()
+
+def ReinitAfterConstruction():
+    instance = PythonSuperInitNone()
+    instance.__init__()
+    return instance
 ";
 
         [OneTimeSetUp]
@@ -88,6 +138,16 @@ public void Setup()
             withArgs_PythonSuperInitInt = pyModule.GetAttr("WithArgs_PythonSuperInitInt");
 
             pureCSharpConstruction = pyModule.GetAttr("PureCSharpConstruction");
+            pureCSharpDefaultConstruction = pyModule.GetAttr("PureCSharpDefaultConstruction");
+            pureCSharpOptionalArgsConstruction = pyModule.GetAttr("PureCSharpOptionalArgsConstruction");
+            pureCSharpOptionalArgsDefaultConstruction = pyModule.GetAttr("PureCSharpOptionalArgsDefaultConstruction");
+            pythonSuperInitDefaultAfterOtherConstruction = pyModule.GetAttr("PythonSuperInitDefaultAfterOtherConstruction");
+            reinitAfterConstruction = pyModule.GetAttr("ReinitAfterConstruction");
+            pythonOptionalArgsSuperInitDefault = pyModule.GetAttr("PythonOptionalArgsSuperInitDefault");
+            pythonOptionalArgsSuperInitValue = pyModule.GetAttr("PythonOptionalArgsSuperInitValue");
+            namespacedSuperInitNone = pyModule.GetAttr("NamespacedSuperInitNone");
+            namespacedSuperInitDefault = pyModule.GetAttr("NamespacedSuperInitDefault");
+            pythonSelectedOverloadInit = pyModule.GetAttr("PythonSelectedOverloadInit");
         }
 
         [OneTimeTearDown]
@@ -127,7 +187,119 @@ public void PureCSharpConstruction()
             {
                 var instance = pureCSharpConstruction();
                 Assert.AreEqual(1, (int)instance.CalledInt);
+                Assert.AreEqual(0, (int)instance.CalledDefault);
+            }
+        }
+
+        [Test]
+        public void PureCSharpDefaultConstruction()
+        {
+            using (Py.GIL())
+            {
+                var instance = pureCSharpDefaultConstruction();
+                Assert.AreEqual(0, (int)instance.CalledInt);
+                Assert.AreEqual(1, (int)instance.CalledDefault);
+            }
+        }
+
+        [Test]
+        public void PureCSharpOptionalArgsConstruction()
+        {
+            using (Py.GIL())
+            {
+                var instance = pureCSharpOptionalArgsConstruction();
+                Assert.AreEqual(1, (int)instance.Calls);
+                Assert.AreEqual(5, (int)instance.Value);
+            }
+        }
+
+        [Test]
+        public void PureCSharpOptionalArgsDefaultConstruction()
+        {
+            using (Py.GIL())
+            {
+                var instance = pureCSharpOptionalArgsDefaultConstruction();
+                Assert.AreEqual(1, (int)instance.Calls);
+                Assert.AreEqual(0, (int)instance.Value);
+            }
+        }
+
+        [Test]
+        public void NoArgs_DefaultConstructorAfterOtherConstruction()
+        {
+            using (Py.GIL())
+            {
+                var instance = pythonSuperInitDefaultAfterOtherConstruction();
+                Assert.AreEqual(0, (int)instance.CalledInt);
                 Assert.AreEqual(1, (int)instance.CalledDefault);
+                Assert.AreEqual(1, (int)instance.other.CalledDefault);
+            }
+        }
+
+        [Test]
+        public void OptionalArgs_DefaultConstructor()
+        {
+            using (Py.GIL())
+            {
+                var instance = pythonOptionalArgsSuperInitDefault();
+                Assert.AreEqual(1, (int)instance.Calls);
+                Assert.AreEqual(0, (int)instance.Value);
+            }
+        }
+
+        [Test]
+        public void OptionalArgs_ValueConstructor()
+        {
+            using (Py.GIL())
+            {
+                var instance = pythonOptionalArgsSuperInitValue();
+                // the base constructor runs in tp_new before __init__, then again with the given value
+                Assert.AreEqual(2, (int)instance.Calls);
+                Assert.AreEqual(5, (int)instance.Value);
+            }
+        }
+
+        [Test]
+        public void SelectedOverload_OptionalArgsConstructor()
+        {
+            using (Py.GIL())
+            {
+                var instance = pythonSelectedOverloadInit();
+                // tp_new ran the parameterless constructor; the explicitly selected one must still run
+                Assert.AreEqual("optional", (string)instance.Selected);
+            }
+        }
+
+        [Test]
+        public void Namespaced_NoInit()
+        {
+            using (Py.GIL())
+            {
+                var instance = namespacedSuperInitNone();
+                Assert.AreEqual(0, (int)instance.CalledInt);
+                Assert.AreEqual(1, (int)instance.CalledDefault);
+            }
+        }
+
+        [Test]
+        public void Namespaced_DefaultConstructor()
+        {
+            using (Py.GIL())
+            {
+                var instance = namespacedSuperInitDefault();
+                Assert.AreEqual(0, (int)instance.CalledInt);
+                Assert.AreEqual(1, (int)instance.CalledDefault);
+            }
+        }
+
+        [Test]
+        public void ReinitAfterConstructionRunsConstructorAgain()
+        {
+            using (Py.GIL())
+            {
+                var instance = reinitAfterConstruction();
+                Assert.AreEqual(0, (int)instance.CalledInt);
+                Assert.AreEqual(2, (int)instance.CalledDefault);
             }
         }
 
@@ -161,7 +333,7 @@ public void WithArgs_DefaultConstructor()
             {
                 var instance = withArgs_PythonSuperInitDefault(1);
                 Assert.AreEqual(0, (int)instance.CalledInt);
-                Assert.AreEqual(2, (int)instance.CalledDefault);
+                Assert.AreEqual(1, (int)instance.CalledDefault);
             }
         }
 
@@ -196,7 +368,7 @@ public void NoArgs_DefaultConstructor()
             {
                 var instance = pythonSuperInitNone();
                 Assert.AreEqual(0, (int)instance.CalledInt);
-                Assert.AreEqual(2, (int)instance.CalledDefault);
+                Assert.AreEqual(1, (int)instance.CalledDefault);
             }
         }
 
@@ -208,7 +380,7 @@ public void NoArgs_NoConstructor()
                 var instance = pythonSuperInitDefault.Invoke();
 
                 Assert.AreEqual(0, (int)instance.CalledInt);
-                Assert.AreEqual(2, (int)instance.CalledDefault);
+                Assert.AreEqual(1, (int)instance.CalledDefault);
             }
         }
     }
@@ -246,6 +418,30 @@ public SuperInit()
         }
     }
 
+    public class OptionalArgsInit
+    {
+        public int Calls { get; private set; }
+        public int Value { get; private set; }
+        public OptionalArgsInit(int value = 0)
+        {
+            Calls++;
+            Value = value;
+        }
+    }
+
+    public class SelectedOverloadInit
+    {
+        public string Selected { get; private set; }
+        public SelectedOverloadInit()
+        {
+            Selected = "default";
+        }
+        public SelectedOverloadInit(string value = "optional")
+        {
+            Selected = value;
+        }
+    }
+
     public class Insight
     {
         public string info;

src/runtime/MethodBinder.cs

@@ -1107,6 +1107,37 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a
                 return default;
             }
 
+            if (binding.info is ConstructorInfo && ManagedType.GetManagedObject(inst) is CLRObject co)
+            {
+                if (co.awaitingCtor)
+                {
+                    // tp_new suppressed finalization until a constructor runs; register it before the
+                    // constructor body, as for a .NET 'new', so the object is finalized even if it throws.
+                    co.awaitingCtor = false;
+                    GC.ReRegisterForFinalize(co.inst);
+                }
+                else if (co.ctorRanInNew != null)
+                {
+                    var ranInNew = co.ctorRanInNew;
+                    co.ctorRanInNew = null;
+                    // A zero-argument base __init__() call that binds the constructor tp_new already ran on this
+                    // Python subclass instance must not run it twice. A __namespace__ subclass's generated type
+                    // mirrors each base constructor and chains to it, so its parameterless constructor has
+                    // already run the base type's parameterless one.
+                    var ranInNewAlready = binding.info == ranInNew
+                        || co.inst is IPythonDerivedType
+                            && binding.info.DeclaringType == ranInNew.DeclaringType.BaseType
+                            && binding.args.Length == 0
+                            && ranInNew.GetParameters().Length == 0;
+                    if (ranInNewAlready
+                        && Runtime.PyTuple_Size(args) == 0
+                        && (kw == null || Runtime.PyDict_Size(kw) == 0))
+                    {
+                        return new NewReference(Runtime.PyNone);
+                    }
+                }
+            }
+
             if (allow_threads)
             {
                 ts = PythonEngine.BeginAllowThreads();

src/runtime/Types/ClassBase.cs

@@ -711,7 +711,14 @@ public override bool Init(BorrowedReference obj, BorrowedReference args, Borrowe
                 // initialization must be done in tp_new
                 return true;
 
-            return base.Init(obj, args, kw);
+            var initialized = base.Init(obj, args, kw);
+            // tp_new's marks only apply to this construction's __init__
+            if (GetManagedObject(obj) is CLRObject co)
+            {
+                co.ctorRanInNew = null;
+                co.awaitingCtor = false;
+            }
+            return initialized;
         }
 
         protected virtual void OnDeserialization(object sender)

src/runtime/Types/ClassObject.cs

@@ -113,6 +113,21 @@ static NewReference tp_new_impl(BorrowedReference tp, BorrowedReference args, Bo
             object obj = FormatterServices.GetUninitializedObject(type);
             var pythonObj = self.NewObjectToPython(obj, tp);
 
+            // Constructing the reflected type itself always continues with __init__ (MetaType.tp_call),
+            // which binds and runs the matching constructor: running one here too would run a constructor
+            // twice on the same object. A Python subclass's __init__ may never call the base __init__, so
+            // it still gets the base constructor here, before its own __init__ runs; that includes a subclass
+            // declaring __namespace__, whose generated .NET type is registered as its own reflected type.
+            if (tp == ClassManager.GetClass(type).Reference && self is not ClassDerivedObject)
+            {
+                GC.SuppressFinalize(obj);
+                if (GetManagedObject(pythonObj.Borrow()) is CLRObject newObject)
+                {
+                    newObject.awaitingCtor = true;
+                }
+                return pythonObj;
+            }
+
             try
             {
                 var binder = new MethodBinder();
@@ -126,6 +141,10 @@ static NewReference tp_new_impl(BorrowedReference tp, BorrowedReference args, Bo
                 if (binding != null)
                 {
                     binding.info.Invoke(obj, BindingFlags.Default, null, binding.args, null);
+                    if (GetManagedObject(pythonObj.Borrow()) is CLRObject co)
+                    {
+                        co.ctorRanInNew = binding.info;
+                    }
                 }
             }
             catch (Exception)

src/runtime/Types/ClrObject.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Reflection;
 using System.Runtime.InteropServices;
 
 namespace Python.Runtime
@@ -10,6 +11,11 @@ namespace Python.Runtime
     internal sealed class CLRObject : ManagedType
     {
         internal readonly object inst;
+        // The zero-argument constructor tp_new ran on this object before __init__, if any.
+        internal MethodBase ctorRanInNew;
+        // Set when tp_new returned this object without running a constructor: its finalization stays
+        // suppressed until one runs, so a failed __init__ doesn't leave it to its finalizer uninitialized.
+        internal bool awaitingCtor;
 
         static NewReference Create(object ob, BorrowedReference tp)
         {