pythonnet · lostmsu · Jan 4, 2022 · Dec 29, 2021
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -46,6 +46,9 @@ details about the cause of the failure
 -   floating point values passed from Python are no longer silently truncated
 when .NET expects an integer [#1342][i1342]
 -   More specific error messages for method argument mismatch
+-   BREAKING: when inheriting from .NET types in Python if you override `__init__` you
+must explicitly call base constructor using `super().__init__(.....)`. Not doing so will lead
+to undefined behavior.
 -   BREAKING: most `PyScope` methods will never return `null`. Instead, `PyObject` `None` will be returned.
 -   BREAKING: `PyScope` was renamed to `PyModule`
 -   BREAKING: Methods with `ref` or `out` parameters and void return type return a tuple of only the `ref` and `out` parameters.
@@ -85,6 +88,7 @@ Instead, `PyIterable` does that.
 ### Fixed
 
 -   Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash
+-   Fixed parameterless .NET constructor being silently called when a matching constructor overload is not found ([#238][i238])
 -   Fix incorrect dereference in params array handling
 -   Fixes issue with function resolution when calling overloaded function with keyword arguments from python ([#1097][i1097])
 -   Fix `object[]` parameters taking precedence when should not in overload resolution
@@ -874,3 +878,4 @@ This version improves performance on benchmarks significantly compared to 2.3.
 [p534]: https://github.com/pythonnet/pythonnet/pull/534
 [i449]: https://github.com/pythonnet/pythonnet/issues/449
 [i1342]: https://github.com/pythonnet/pythonnet/issues/1342
+[i238]: https://github.com/pythonnet/pythonnet/issues/238
diff --git a/src/embed_tests/StateSerialization/MethodSerialization.cs b/src/embed_tests/StateSerialization/MethodSerialization.cs
@@ -19,6 +19,16 @@ public void GenericRoundtrip()
         Assert.AreEqual(method, restored.Value);
     }
 
+    [Test]
+    public void ConstrctorRoundtrip()
+    {
+        var ctor = typeof(MethodTestHost).GetConstructor(new[] { typeof(int) });
+        var maybeConstructor = new MaybeMethodBase<MethodBase>(ctor);
+        var restored = SerializationRoundtrip(maybeConstructor);
+        Assert.IsTrue(restored.Valid);
+        Assert.AreEqual(ctor, restored.Value);
+    }
+
     static T SerializationRoundtrip<T>(T item)
     {
         using var buf = new MemoryStream();
@@ -31,5 +41,6 @@ static T SerializationRoundtrip<T>(T item)
 
 public class MethodTestHost
 {
+    public MethodTestHost(int _) { }
     public void Generic<T>(T item, T[] array, ref T @ref) { }
 }
diff --git a/src/runtime/NewReference.cs b/src/runtime/NewReference.cs
@@ -41,6 +41,17 @@ public PyObject MoveToPyObject()
             return new PyObject(this.StealNullable());
         }
 
+        /// <summary>
+        /// Creates new instance of <see cref="NewReference"/> which now owns the pointer.
+        /// Sets the original reference to <c>null</c>, as it no longer owns the pointer.
+        /// </summary>
+        public NewReference Move()
+        {
+            var result = new NewReference(this);
+            this.pointer = default;
+            return result;
+        }
+
         /// <summary>Moves ownership of this instance to unmanged pointer</summary>
         public IntPtr DangerousMoveToPointer()
         {

diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs
@@ -556,6 +556,17 @@ public virtual void InitializeSlots(BorrowedReference pyType, SlotsHolder slotsH
             }
         }
 
+        public virtual bool HasCustomNew() => this.GetType().GetMethod("tp_new") is not null;
+
+        public override bool Init(BorrowedReference obj, BorrowedReference args, BorrowedReference kw)
+        {
+            if (this.HasCustomNew())
+                // initialization must be done in tp_new
+                return true;
+
+            return base.Init(obj, args, kw);
+        }
+
         protected virtual void OnDeserialization(object sender)
         {
             this.dotNetMembers = new List<string>();

diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs
@@ -50,23 +50,26 @@ internal ClassDerivedObject(Type tp) : base(tp)
         {
         }
 
-        /// <summary>
-        /// Implements __new__ for derived classes of reflected classes.
-        /// </summary>
-        public new static NewReference tp_new(BorrowedReference tp, BorrowedReference args, BorrowedReference kw)
+        protected override NewReference NewObjectToPython(object obj, BorrowedReference tp)
         {
-            var cls = (ClassDerivedObject)GetManagedObject(tp)!;
+            var self = base.NewObjectToPython(obj, tp);
 
-            // call the managed constructor
-            object? obj = cls.binder.InvokeRaw(null, args, kw);
-            if (obj == null)
+            SetPyObj((IPythonDerivedType)obj, self.Borrow());
+
+            // 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.
+            if (!self.IsNull())
             {
-                return default;
+                Runtime.XDecref(self.Steal());
             }
 
-            // return the pointer to the python object
-            // (this indirectly calls ClassDerivedObject.ToPython)
-            return Converter.ToPython(obj, cls.GetType());
+            return Converter.ToPython(obj, type.Value);
+        }
+
+        protected override void SetTypeNewSlot(BorrowedReference pyType, SlotsHolder slotsHolder)
+        {
+            // Python derived types rely on base tp_new and overridden __init__
         }
 
         public new static void tp_dealloc(NewReference ob)
@@ -824,37 +827,24 @@ public static void InvokeSetProperty<T>(IPythonDerivedType obj, string propertyN
 
         public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, object[] args)
         {
+            var selfRef = GetPyObj(obj);
+            if (selfRef.Ref == null)
+            {
+                // this might happen when the object is created from .NET
+                using var _ = Py.GIL();
+                // In the end we 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.
+                using var self = CLRObject.GetReference(obj, obj.GetType());
+                SetPyObj(obj, self.Borrow());
+            }
+
             // call the base constructor
             obj.GetType().InvokeMember(origCtorName,
                 BindingFlags.InvokeMethod,
                 null,
                 obj,
                 args);
-
-            NewReference self = default;
-            PyGILState gs = Runtime.PyGILState_Ensure();
-            try
-            {
-                // create the python object
-                var type = ClassManager.GetClass(obj.GetType());
-                self = CLRObject.GetReference(obj, type);
-
-                // set __pyobj__ to self and deref the python object which will allow this
-                // object to be collected.
-                SetPyObj(obj, self.Borrow());
-            }
-            finally
-            {
-                // 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.
-                if (!self.IsNull())
-                {
-                    Runtime.XDecref(self.Steal());
-                }
-
-                Runtime.PyGILState_Release(gs);
-            }
         }
 
         public static void PyFinalize(IPythonDerivedType obj)
@@ -890,7 +880,7 @@ internal static UnsafeReferenceWithRun GetPyObj(IPythonDerivedType obj)
             return (UnsafeReferenceWithRun)fi.GetValue(obj);
         }
 
-        static void SetPyObj(IPythonDerivedType obj, BorrowedReference pyObj)
+        internal static void SetPyObj(IPythonDerivedType obj, BorrowedReference pyObj)
         {
             FieldInfo fi = GetPyObjField(obj.GetType())!;
             fi.SetValue(obj, new UnsafeReferenceWithRun(pyObj));

diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs
@@ -210,7 +210,7 @@ internal static void InitClassBase(Type type, ClassBase impl, ReflectedClrType p
             // information, including generating the member descriptors
             // that we'll be putting in the Python class __dict__.
 
-            ClassInfo info = GetClassInfo(type);
+            ClassInfo info = GetClassInfo(type, impl);
 
             impl.indexer = info.indexer;
             impl.richcompare.Clear();
@@ -252,16 +252,17 @@ internal static void InitClassBase(Type type, ClassBase impl, ReflectedClrType p
             // required that the ClassObject.ctors be changed to internal
             if (co != null)
             {
-                if (co.NumCtors > 0)
+                if (co.NumCtors > 0 && !co.HasCustomNew())
                 {
                     // Implement Overloads on the class object
                     if (!CLRModule._SuppressOverloads)
                     {
-                        using var ctors = new ConstructorBinding(type, pyType, co.binder).Alloc();
-                        // ExtensionType types are untracked, so don't Incref() them.
+                        // HACK: __init__ points to instance constructors.
+                        // When unbound they fully instantiate object, so we get overloads for free from MethodBinding.
+                        var init = info.members["__init__"];
                         // TODO: deprecate __overloads__ soon...
-                        Runtime.PyDict_SetItem(dict, PyIdentifier.__overloads__, ctors.Borrow());
-                        Runtime.PyDict_SetItem(dict, PyIdentifier.Overloads, ctors.Borrow());
+                        Runtime.PyDict_SetItem(dict, PyIdentifier.__overloads__, init);
+                        Runtime.PyDict_SetItem(dict, PyIdentifier.Overloads, init);
                     }
 
                     // don't generate the docstring if one was already set from a DocStringAttribute.
@@ -320,10 +321,10 @@ internal static bool ShouldBindEvent(EventInfo ei)
             return ShouldBindMethod(ei.GetAddMethod(true));
         }
 
-        private static ClassInfo GetClassInfo(Type type)
+        private static ClassInfo GetClassInfo(Type type, ClassBase impl)
         {
             var ci = new ClassInfo();
-            var methods = new Dictionary<string, List<MethodInfo>>();
+            var methods = new Dictionary<string, List<MethodBase>>();
             MethodInfo meth;
             ExtensionType ob;
             string name;
@@ -420,13 +421,33 @@ private static ClassInfo GetClassInfo(Type type)
                             continue;
                         }
                         name = meth.Name;
+
+                        //TODO mangle?
+                        if (name == "__init__" && !impl.HasCustomNew())
+                            continue;
+
                         if (!methods.TryGetValue(name, out var methodList))
                         {
-                            methodList = methods[name] = new List<MethodInfo>();
+                            methodList = methods[name] = new List<MethodBase>();
                         }
                         methodList.Add(meth);
                         continue;
 
+                    case MemberTypes.Constructor when !impl.HasCustomNew():
+                        var ctor = (ConstructorInfo)mi;
+                        if (ctor.IsStatic)
+                        {
+                            continue;
+                        }
+
+                        name = "__init__";
+                        if (!methods.TryGetValue(name, out methodList))
+                        {
+                            methodList = methods[name] = new List<MethodBase>();
+                        }
+                        methodList.Add(ctor);
+                        continue;
+
                     case MemberTypes.Property:
                         var pi = (PropertyInfo)mi;
 

diff --git a/src/runtime/classobject.cs b/src/runtime/classobject.cs
@@ -1,6 +1,8 @@
 using System;
+using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
+using System.Runtime.Serialization;
 
 namespace Python.Runtime
 {
@@ -13,18 +15,12 @@ namespace Python.Runtime
     [Serializable]
     internal class ClassObject : ClassBase
     {
-        internal ConstructorBinder binder;
-        internal int NumCtors = 0;
+        internal readonly int NumCtors = 0;
 
         internal ClassObject(Type tp) : base(tp)
         {
             var _ctors = type.Value.GetConstructors();
             NumCtors = _ctors.Length;
-            binder = new ConstructorBinder(type.Value);
-            foreach (ConstructorInfo t in _ctors)
-            {
-                binder.AddMethod(t);
-            }
         }
 
 
@@ -33,7 +29,12 @@ internal ClassObject(Type tp) : base(tp)
         /// </summary>
         internal NewReference GetDocString()
         {
-            MethodBase[] methods = binder.GetMethods();
+            if (!type.Valid)
+            {
+                return Exceptions.RaiseTypeError(type.DeletedMessage);
+            }
+
+            MethodBase[] methods = type.Value.GetConstructors();
             var str = "";
             foreach (MethodBase t in methods)
             {
@@ -50,7 +51,7 @@ internal NewReference GetDocString()
         /// <summary>
         /// Implements __new__ for reflected classes and value types.
         /// </summary>
-        public static NewReference tp_new(BorrowedReference tp, BorrowedReference args, BorrowedReference kw)
+        static NewReference tp_new_impl(BorrowedReference tp, BorrowedReference args, BorrowedReference kw)
         {
             var self = GetManagedObject(tp) as ClassObject;
 
@@ -100,15 +101,49 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args,
                 return NewEnum(type, args, tp);
             }
 
-            object? obj = self.binder.InvokeRaw(null, args, kw);
-            if (obj == null)
+            if (IsGenericNullable(type))
             {
-                return default;
+                // Nullable<T> has special handling in .NET runtime.
+                // Invoking its constructor via reflection on an uninitialized instance
+                // does not actually set the object fields.
+                return NewNullable(type, args, kw, tp);
             }
 
-            return CLRObject.GetReference(obj, tp);
+            object obj = FormatterServices.GetUninitializedObject(type);
+
+            return self.NewObjectToPython(obj, tp);
+        }
+
+        protected virtual void SetTypeNewSlot(BorrowedReference pyType, SlotsHolder slotsHolder)
+        {
+            TypeManager.InitializeSlotIfEmpty(pyType, TypeOffset.tp_new, new Interop.BBB_N(tp_new_impl), slotsHolder);
+        }
+
+        public override bool HasCustomNew()
+        {
+            if (base.HasCustomNew()) return true;
+
+            Type clrType = type.Value;
+            return clrType.IsPrimitive
+                || clrType.IsEnum
+                || clrType == typeof(string)
+                || IsGenericNullable(clrType);
         }
 
+        static bool IsGenericNullable(Type type)
+            => type.IsValueType && type.IsGenericType
+            && type.GetGenericTypeDefinition() == typeof(Nullable<>);
+
+        public override void InitializeSlots(BorrowedReference pyType, SlotsHolder slotsHolder)
+        {
+            base.InitializeSlots(pyType, slotsHolder);
+
+            this.SetTypeNewSlot(pyType, slotsHolder);
+        }
+
+        protected virtual NewReference NewObjectToPython(object obj, BorrowedReference tp)
+            => CLRObject.GetReference(obj, tp);
+
         private static NewReference NewEnum(Type type, BorrowedReference args, BorrowedReference tp)
         {
             nint argCount = Runtime.PyTuple_Size(args);
@@ -146,6 +181,28 @@ private static NewReference NewEnum(Type type, BorrowedReference args, BorrowedR
             return CLRObject.GetReference(enumValue, tp);
         }
 
+        private static NewReference NewNullable(Type type, BorrowedReference args, BorrowedReference kw, BorrowedReference tp)
+        {
+            Debug.Assert(IsGenericNullable(type));
+
+            if (kw != null)
+            {
+                return Exceptions.RaiseTypeError("System.Nullable<T> constructor does not support keyword arguments");
+            }
+
+            nint argsCount = Runtime.PyTuple_Size(args);
+            if (argsCount != 1)
+            {
+                return Exceptions.RaiseTypeError("System.Nullable<T> constructor expects 1 argument, got " + (int)argsCount);
+            }
+
+            var value = Runtime.PyTuple_GetItem(args, 0);
+            var elementType = type.GetGenericArguments()[0];
+            return Converter.ToManaged(value, elementType, out var result, setError: true)
+                ? CLRObject.GetReference(result!, tp)
+                : default;
+        }
+
 
         /// <summary>
         /// Implementation of [] semantics for reflected types. This exists