Visitar URL original
reworked the way .NET objects are constructed from Python · losttech/pythonnet@6301476 · GitHub
Skip to content

Commit 6301476

Browse files
lostmsulostmsu
committed
reworked the way .NET objects are constructed from Python
was: tp_new implementation would call .NET constructor and return a fully constructed object now: Except for some special .NET types tp_new creates uninitialized .NET object, which is later initialized by calling __init__. __init__ is set using type dictionary to a MethodObject, that contains ConstructorInfo[] instead of MethodInfo[] This allows Python to: 1) freely override __init__ 2) when deriving from .NET types call base __init__ (e.g. .NET constructor), and choose the overload as needed fixes pythonnet#238
1 parent 8d61215 commit 6301476

21 files changed

Lines changed: 293 additions & 481 deletions

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ details about the cause of the failure
4646
- floating point values passed from Python are no longer silently truncated
4747
when .NET expects an integer [#1342][i1342]
4848
- More specific error messages for method argument mismatch
49+
- BREAKING: when inheriting from .NET types in Python if you override `__init__` you
50+
must explicitly call base constructor using `super().__init__(.....)`. Not doing so will lead
51+
to undefined behavior.
4952
- BREAKING: most `PyScope` methods will never return `null`. Instead, `PyObject` `None` will be returned.
5053
- BREAKING: `PyScope` was renamed to `PyModule`
5154
- 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.
8588
### Fixed
8689

8790
- Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash
91+
- Fixed parameterless .NET constructor being silently called when a matching constructor overload is not found ([#238][i238])
8892
- Fix incorrect dereference in params array handling
8993
- Fixes issue with function resolution when calling overloaded function with keyword arguments from python ([#1097][i1097])
9094
- 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.
874878
[p534]: https://github.com/pythonnet/pythonnet/pull/534
875879
[i449]: https://github.com/pythonnet/pythonnet/issues/449
876880
[i1342]: https://github.com/pythonnet/pythonnet/issues/1342
881+
[i238]: https://github.com/pythonnet/pythonnet/issues/238

‎src/embed_tests/StateSerialization/MethodSerialization.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,16 @@ public void GenericRoundtrip()
1919
Assert.AreEqual(method, restored.Value);
2020
}
2121

22+
[Test]
23+
public void ConstrctorRoundtrip()
24+
{
25+
var ctor = typeof(MethodTestHost).GetConstructor(new[] { typeof(int) });
26+
var maybeConstructor = new MaybeMethodBase<MethodBase>(ctor);
27+
var restored = SerializationRoundtrip(maybeConstructor);
28+
Assert.IsTrue(restored.Valid);
29+
Assert.AreEqual(ctor, restored.Value);
30+
}
31+
2232
static T SerializationRoundtrip<T>(T item)
2333
{
2434
using var buf = new MemoryStream();
@@ -31,5 +41,6 @@ static T SerializationRoundtrip<T>(T item)
3141

3242
public class MethodTestHost
3343
{
44+
public MethodTestHost(int _) { }
3445
public void Generic<T>(T item, T[] array, ref T @ref) { }
3546
}

‎src/runtime/NewReference.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@ public PyObject MoveToPyObject()
4141
return new PyObject(this.StealNullable());
4242
}
4343

44+
/// <summary>
45+
/// Creates new instance of <see cref="NewReference"/> which now owns the pointer.
46+
/// Sets the original reference to <c>null</c>, as it no longer owns the pointer.
47+
/// </summary>
48+
public NewReference Move()
49+
{
50+
var result = new NewReference(this);
51+
this.pointer = default;
52+
return result;
53+
}
54+
4455
/// <summary>Moves ownership of this instance to unmanged pointer</summary>
4556
public IntPtr DangerousMoveToPointer()
4657
{

‎src/runtime/classbase.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,16 @@ public virtual void InitializeSlots(BorrowedReference pyType, SlotsHolder slotsH
556556
}
557557
}
558558

559+
public virtual bool ExposesClrConstructors() => this.GetType().GetMethod("tp_new") is null;
560+
561+
public override bool Init(BorrowedReference obj, BorrowedReference args, BorrowedReference kw)
562+
{
563+
if (ExposesClrConstructors())
564+
return base.Init(obj, args, kw);
565+
else
566+
return true;
567+
}
568+
559569
protected virtual void OnDeserialization(object sender)
560570
{
561571
this.dotNetMembers = new List<string>();

‎src/runtime/classderived.cs‎

Lines changed: 25 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -50,25 +50,25 @@ internal ClassDerivedObject(Type tp) : base(tp)
5050
{
5151
}
5252

53-
/// <summary>
54-
/// Implements __new__ for derived classes of reflected classes.
55-
/// </summary>
56-
public new static NewReference tp_new(BorrowedReference tp, BorrowedReference args, BorrowedReference kw)
53+
protected override NewReference NewObjectToPython(object obj, BorrowedReference tp)
5754
{
58-
var cls = (ClassDerivedObject)GetManagedObject(tp)!;
55+
var self = base.NewObjectToPython(obj, tp);
5956

60-
// call the managed constructor
61-
object? obj = cls.binder.InvokeRaw(null, args, kw);
62-
if (obj == null)
57+
SetPyObj((IPythonDerivedType)obj, self.Borrow());
58+
59+
// Decrement the python object's reference count.
60+
// This doesn't actually destroy the object, it just sets the reference to this object
61+
// to be a weak reference and it will be destroyed when the C# object is destroyed.
62+
if (!self.IsNull())
6363
{
64-
return default;
64+
Runtime.XDecref(self.Steal());
6565
}
6666

67-
// return the pointer to the python object
68-
// (this indirectly calls ClassDerivedObject.ToPython)
69-
return Converter.ToPython(obj, cls.GetType());
67+
return Converter.ToPython(obj, type.Value);
7068
}
7169

70+
protected override bool DefinesTypeNew() => false;
71+
7272
public new static void tp_dealloc(NewReference ob)
7373
{
7474
var self = (CLRObject?)GetManagedObject(ob.Borrow());
@@ -824,37 +824,24 @@ public static void InvokeSetProperty<T>(IPythonDerivedType obj, string propertyN
824824

825825
public static void InvokeCtor(IPythonDerivedType obj, string origCtorName, object[] args)
826826
{
827+
var selfRef = GetPyObj(obj);
828+
if (selfRef.Ref == null)
829+
{
830+
// this might happen when the object is created from .NET
831+
using var _ = Py.GIL();
832+
// In the end we decrement the python object's reference count.
833+
// This doesn't actually destroy the object, it just sets the reference to this object
834+
// to be a weak reference and it will be destroyed when the C# object is destroyed.
835+
using var self = CLRObject.GetReference(obj, obj.GetType());
836+
SetPyObj(obj, self.Borrow());
837+
}
838+
827839
// call the base constructor
828840
obj.GetType().InvokeMember(origCtorName,
829841
BindingFlags.InvokeMethod,
830842
null,
831843
obj,
832844
args);
833-
834-
NewReference self = default;
835-
PyGILState gs = Runtime.PyGILState_Ensure();
836-
try
837-
{
838-
// create the python object
839-
var type = ClassManager.GetClass(obj.GetType());
840-
self = CLRObject.GetReference(obj, type);
841-
842-
// set __pyobj__ to self and deref the python object which will allow this
843-
// object to be collected.
844-
SetPyObj(obj, self.Borrow());
845-
}
846-
finally
847-
{
848-
// Decrement the python object's reference count.
849-
// This doesn't actually destroy the object, it just sets the reference to this object
850-
// to be a weak reference and it will be destroyed when the C# object is destroyed.
851-
if (!self.IsNull())
852-
{
853-
Runtime.XDecref(self.Steal());
854-
}
855-
856-
Runtime.PyGILState_Release(gs);
857-
}
858845
}
859846

860847
public static void PyFinalize(IPythonDerivedType obj)
@@ -890,7 +877,7 @@ internal static UnsafeReferenceWithRun GetPyObj(IPythonDerivedType obj)
890877
return (UnsafeReferenceWithRun)fi.GetValue(obj);
891878
}
892879

893-
static void SetPyObj(IPythonDerivedType obj, BorrowedReference pyObj)
880+
internal static void SetPyObj(IPythonDerivedType obj, BorrowedReference pyObj)
894881
{
895882
FieldInfo fi = GetPyObjField(obj.GetType())!;
896883
fi.SetValue(obj, new UnsafeReferenceWithRun(pyObj));

‎src/runtime/classmanager.cs‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ internal static void InitClassBase(Type type, ClassBase impl, ReflectedClrType p
210210
// information, including generating the member descriptors
211211
// that we'll be putting in the Python class __dict__.
212212

213-
ClassInfo info = GetClassInfo(type);
213+
ClassInfo info = GetClassInfo(type, impl);
214214

215215
impl.indexer = info.indexer;
216216
impl.richcompare.Clear();
@@ -252,16 +252,17 @@ internal static void InitClassBase(Type type, ClassBase impl, ReflectedClrType p
252252
// required that the ClassObject.ctors be changed to internal
253253
if (co != null)
254254
{
255-
if (co.NumCtors > 0)
255+
if (co.NumCtors > 0 && co.ExposesClrConstructors())
256256
{
257257
// Implement Overloads on the class object
258258
if (!CLRModule._SuppressOverloads)
259259
{
260-
using var ctors = new ConstructorBinding(type, pyType, co.binder).Alloc();
261-
// ExtensionType types are untracked, so don't Incref() them.
260+
// HACK: __init__ points to instance constructors.
261+
// When unbound they fully instantiate object, so we get overloads for free from MethodBinding.
262+
var init = info.members["__init__"];
262263
// TODO: deprecate __overloads__ soon...
263-
Runtime.PyDict_SetItem(dict, PyIdentifier.__overloads__, ctors.Borrow());
264-
Runtime.PyDict_SetItem(dict, PyIdentifier.Overloads, ctors.Borrow());
264+
Runtime.PyDict_SetItem(dict, PyIdentifier.__overloads__, init);
265+
Runtime.PyDict_SetItem(dict, PyIdentifier.Overloads, init);
265266
}
266267

267268
// don't generate the docstring if one was already set from a DocStringAttribute.
@@ -320,10 +321,10 @@ internal static bool ShouldBindEvent(EventInfo ei)
320321
return ShouldBindMethod(ei.GetAddMethod(true));
321322
}
322323

323-
private static ClassInfo GetClassInfo(Type type)
324+
private static ClassInfo GetClassInfo(Type type, ClassBase impl)
324325
{
325326
var ci = new ClassInfo();
326-
var methods = new Dictionary<string, List<MethodInfo>>();
327+
var methods = new Dictionary<string, List<MethodBase>>();
327328
MethodInfo meth;
328329
ExtensionType ob;
329330
string name;
@@ -420,13 +421,33 @@ private static ClassInfo GetClassInfo(Type type)
420421
continue;
421422
}
422423
name = meth.Name;
424+
425+
#warning mangle?
426+
if (name == "__init__" && impl.ExposesClrConstructors())
427+
continue;
428+
423429
if (!methods.TryGetValue(name, out var methodList))
424430
{
425-
methodList = methods[name] = new List<MethodInfo>();
431+
methodList = methods[name] = new List<MethodBase>();
426432
}
427433
methodList.Add(meth);
428434
continue;
429435

436+
case MemberTypes.Constructor when impl.ExposesClrConstructors():
437+
var ctor = (ConstructorInfo)mi;
438+
if (ctor.IsStatic)
439+
{
440+
continue;
441+
}
442+
443+
name = "__init__";
444+
if (!methods.TryGetValue(name, out methodList))
445+
{
446+
methodList = methods[name] = new List<MethodBase>();
447+
}
448+
methodList.Add(ctor);
449+
continue;
450+
430451
case MemberTypes.Property:
431452
var pi = (PropertyInfo)mi;
432453

‎src/runtime/classobject.cs‎

Lines changed: 76 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
using System;
2+
using System.Diagnostics;
23
using System.Linq;
34
using System.Reflection;
5+
using System.Runtime.Serialization;
46

57
namespace Python.Runtime
68
{
@@ -13,18 +15,12 @@ namespace Python.Runtime
1315
[Serializable]
1416
internal class ClassObject : ClassBase
1517
{
16-
internal ConstructorBinder binder;
17-
internal int NumCtors = 0;
18+
internal readonly int NumCtors = 0;
1819

1920
internal ClassObject(Type tp) : base(tp)
2021
{
2122
var _ctors = type.Value.GetConstructors();
2223
NumCtors = _ctors.Length;
23-
binder = new ConstructorBinder(type.Value);
24-
foreach (ConstructorInfo t in _ctors)
25-
{
26-
binder.AddMethod(t);
27-
}
2824
}
2925

3026

@@ -33,7 +29,12 @@ internal ClassObject(Type tp) : base(tp)
3329
/// </summary>
3430
internal NewReference GetDocString()
3531
{
36-
MethodBase[] methods = binder.GetMethods();
32+
if (!type.Valid)
33+
{
34+
return Exceptions.RaiseTypeError(type.DeletedMessage);
35+
}
36+
37+
MethodBase[] methods = type.Value.GetConstructors();
3738
var str = "";
3839
foreach (MethodBase t in methods)
3940
{
@@ -50,7 +51,7 @@ internal NewReference GetDocString()
5051
/// <summary>
5152
/// Implements __new__ for reflected classes and value types.
5253
/// </summary>
53-
public static NewReference tp_new(BorrowedReference tp, BorrowedReference args, BorrowedReference kw)
54+
static NewReference tp_new_impl(BorrowedReference tp, BorrowedReference args, BorrowedReference kw)
5455
{
5556
var self = GetManagedObject(tp) as ClassObject;
5657

@@ -100,15 +101,50 @@ public static NewReference tp_new(BorrowedReference tp, BorrowedReference args,
100101
return NewEnum(type, args, tp);
101102
}
102103

103-
object? obj = self.binder.InvokeRaw(null, args, kw);
104-
if (obj == null)
104+
if (IsGenericNullable(type))
105105
{
106-
return default;
106+
// Nullable<T> has special handling in .NET runtime.
107+
// Invoking its constructor via reflection on an uninitialized instance
108+
// does not actually set the object fields.
109+
object? nullable = NewNullable(type, args, kw);
110+
return nullable is null ? default : CLRObject.GetReference(nullable, tp);
107111
}
108112

109-
return CLRObject.GetReference(obj, tp);
113+
object obj = FormatterServices.GetUninitializedObject(type);
114+
115+
return self.NewObjectToPython(obj, tp);
110116
}
111117

118+
protected virtual bool DefinesTypeNew() => true;
119+
120+
public override bool ExposesClrConstructors()
121+
{
122+
if (!base.ExposesClrConstructors()) return false;
123+
124+
Type clrType = type.Value;
125+
return !clrType.IsPrimitive
126+
&& !clrType.IsEnum
127+
&& clrType != typeof(string)
128+
&& !IsGenericNullable(clrType);
129+
}
130+
131+
static bool IsGenericNullable(Type type)
132+
=> type.IsValueType && type.IsGenericType
133+
&& type.GetGenericTypeDefinition() == typeof(Nullable<>);
134+
135+
public override void InitializeSlots(BorrowedReference pyType, SlotsHolder slotsHolder)
136+
{
137+
base.InitializeSlots(pyType, slotsHolder);
138+
139+
if (DefinesTypeNew())
140+
{
141+
TypeManager.InitializeSlotIfEmpty(pyType, TypeOffset.tp_new, new Interop.BBB_N(tp_new_impl), slotsHolder);
142+
}
143+
}
144+
145+
protected virtual NewReference NewObjectToPython(object obj, BorrowedReference tp)
146+
=> CLRObject.GetReference(obj, tp);
147+
112148
private static NewReference NewEnum(Type type, BorrowedReference args, BorrowedReference tp)
113149
{
114150
nint argCount = Runtime.PyTuple_Size(args);
@@ -146,6 +182,33 @@ private static NewReference NewEnum(Type type, BorrowedReference args, BorrowedR
146182
return CLRObject.GetReference(enumValue, tp);
147183
}
148184

185+
private static object? NewNullable(Type type, BorrowedReference args, BorrowedReference kw)
186+
{
187+
Debug.Assert(IsGenericNullable(type));
188+
189+
if (kw != null)
190+
{
191+
Exceptions.RaiseTypeError("System.Nullable<T> constructor does not support keyword arguments");
192+
return null;
193+
}
194+
195+
nint argsCount = Runtime.PyTuple_Size(args);
196+
if (argsCount == 0)
197+
return Activator.CreateInstance(type);
198+
199+
if (argsCount != 1)
200+
{
201+
Exceptions.RaiseTypeError("System.Nullable<T> constructor expects 1 argument, got " + (int)argsCount);
202+
return null;
203+
}
204+
205+
var value = Runtime.PyTuple_GetItem(args, 0);
206+
var elementType = type.GetGenericArguments()[0];
207+
return Converter.ToManaged(value, elementType, out var result, setError: true)
208+
? Activator.CreateInstance(type, result)
209+
: null;
210+
}
211+
149212

150213
/// <summary>
151214
/// Implementation of [] semantics for reflected types. This exists

0 commit comments

Comments
 (0)