From a07f00ced33881a929d82acb11a798eaded2cbf3 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Tue, 28 Dec 2021 12:50:32 -0800 Subject: [PATCH] base accessors were not exposed to Python because .NET PropertyInfo GetMethod would not return base non-overriden accessor for a partially overriden property because of that when constructing PropertyObject we scan base classes to find base accessor (if any) this might have performance implications due to replacement of PropertyInfo.GetValue with getter.Invoke (not tested) fixes https://github.com/pythonnet/pythonnet/issues/1455 --- src/embed_tests/Inheritance.cs | 21 +++++++++++++ src/runtime/ReflectionUtil.cs | 56 ++++++++++++++++++++++++++++++++++ src/runtime/propertyobject.cs | 35 +++++++++++++++------ 3 files changed, 103 insertions(+), 9 deletions(-) create mode 100644 src/runtime/ReflectionUtil.cs diff --git a/src/embed_tests/Inheritance.cs b/src/embed_tests/Inheritance.cs index a992ee8e4..ebbc24dc4 100644 --- a/src/embed_tests/Inheritance.cs +++ b/src/embed_tests/Inheritance.cs @@ -119,6 +119,15 @@ public void BaseClearIsCalled() scope.Set("exn", null); Assert.AreEqual(1, msg.Refcount); } + + // https://github.com/pythonnet/pythonnet/issues/1455 + [Test] + public void PropertyAccessorOverridden() + { + using var derived = new PropertyAccessorDerived().ToPython(); + derived.SetAttr(nameof(PropertyAccessorDerived.VirtualProp), "hi".ToPython()); + Assert.AreEqual("HI", derived.GetAttr(nameof(PropertyAccessorDerived.VirtualProp)).As()); + } } class ExtraBaseTypeProvider : IPythonBaseTypeProvider @@ -192,6 +201,18 @@ public int XProp } } + public class PropertyAccessorBase + { + public virtual string VirtualProp { get; set; } + } + + public class PropertyAccessorIntermediate: PropertyAccessorBase { } + + public class PropertyAccessorDerived: PropertyAccessorIntermediate + { + public override string VirtualProp { set => base.VirtualProp = value.ToUpperInvariant(); } + } + public class ContainerClass { public void BaseMethod() { } diff --git a/src/runtime/ReflectionUtil.cs b/src/runtime/ReflectionUtil.cs new file mode 100644 index 000000000..58d0a506e --- /dev/null +++ b/src/runtime/ReflectionUtil.cs @@ -0,0 +1,56 @@ +namespace Python.Runtime; + +using System; +using System.Reflection; + +static class ReflectionUtil +{ + public static MethodInfo? GetBaseGetMethod(this PropertyInfo property, bool nonPublic) + { + if (property is null) throw new ArgumentNullException(nameof(property)); + + Type baseType = property.DeclaringType.BaseType; + BindingFlags bindingFlags = property.GetBindingFlags(); + + while (baseType is not null) + { + var baseProperty = baseType.GetProperty(property.Name, bindingFlags | BindingFlags.DeclaredOnly); + var accessor = baseProperty?.GetGetMethod(nonPublic); + if (accessor is not null) + return accessor; + + baseType = baseType.BaseType; + } + + return null; + } + + public static MethodInfo? GetBaseSetMethod(this PropertyInfo property, bool nonPublic) + { + if (property is null) throw new ArgumentNullException(nameof(property)); + + Type baseType = property.DeclaringType.BaseType; + BindingFlags bindingFlags = property.GetBindingFlags(); + + while (baseType is not null) + { + var baseProperty = baseType.GetProperty(property.Name, bindingFlags | BindingFlags.DeclaredOnly); + var accessor = baseProperty?.GetSetMethod(nonPublic); + if (accessor is not null) + return accessor; + + baseType = baseType.BaseType; + } + + return null; + } + + public static BindingFlags GetBindingFlags(this PropertyInfo property) + { + var accessor = property.GetMethod ?? property.SetMethod; + BindingFlags flags = default; + flags |= accessor.IsStatic ? BindingFlags.Static : BindingFlags.Instance; + flags |= accessor.IsPublic ? BindingFlags.Public : BindingFlags.NonPublic; + return flags; + } +} diff --git a/src/runtime/propertyobject.cs b/src/runtime/propertyobject.cs index 140bd47b5..f09d1696a 100644 --- a/src/runtime/propertyobject.cs +++ b/src/runtime/propertyobject.cs @@ -1,5 +1,6 @@ using System; using System.Reflection; +using System.Runtime.Serialization; namespace Python.Runtime { @@ -8,17 +9,25 @@ namespace Python.Runtime /// Implements a Python descriptor type that manages CLR properties. /// [Serializable] - internal class PropertyObject : ExtensionType + internal class PropertyObject : ExtensionType, IDeserializationCallback { internal MaybeMemberInfo info; - private MaybeMethodInfo getter; - private MaybeMethodInfo setter; + [NonSerialized] + private MethodInfo? getter; + [NonSerialized] + private MethodInfo? setter; public PropertyObject(PropertyInfo md) { - getter = md.GetGetMethod(true); - setter = md.GetSetMethod(true); info = new MaybeMemberInfo(md); + CacheAccessors(); + } + + void CacheAccessors() + { + PropertyInfo md = info.Value; + getter = md.GetGetMethod(true) ?? md.GetBaseGetMethod(true); + setter = md.GetSetMethod(true) ?? md.GetBaseSetMethod(true); } @@ -35,7 +44,7 @@ public static NewReference tp_descr_get(BorrowedReference ds, BorrowedReference return Exceptions.RaiseTypeError(self.info.DeletedMessage); } var info = self.info.Value; - MethodInfo getter = self.getter.UnsafeValue; + MethodInfo? getter = self.getter; object result; @@ -70,7 +79,7 @@ public static NewReference tp_descr_get(BorrowedReference ds, BorrowedReference try { - result = info.GetValue(co.inst, null); + result = getter.Invoke(co.inst, Array.Empty()); return Converter.ToPython(result, info.PropertyType); } catch (Exception e) @@ -100,7 +109,7 @@ public static int tp_descr_set(BorrowedReference ds, BorrowedReference ob, Borro } var info = self.info.Value; - MethodInfo setter = self.setter.UnsafeValue; + MethodInfo? setter = self.setter; if (val == null) { @@ -141,7 +150,7 @@ public static int tp_descr_set(BorrowedReference ds, BorrowedReference ob, Borro Exceptions.RaiseTypeError("invalid target"); return -1; } - info.SetValue(co.inst, newval, null); + setter.Invoke(co.inst, new object?[] { newval }); } else { @@ -169,5 +178,13 @@ public static NewReference tp_repr(BorrowedReference ob) var self = (PropertyObject)GetManagedObject(ob)!; return Runtime.PyString_FromString($""); } + + void IDeserializationCallback.OnDeserialization(object sender) + { + if (info.Valid) + { + CacheAccessors(); + } + } } }