Fix attribute setting. (#293)

filmor · web-flow · commit fb23dd174a20 · 2016-11-25T13:00:01.000+01:00
- When modifying attributes on a class, PyType_Modified has to be called
- The result of a lookup may be non-NULL without being a descriptor (e.g. if we
  set it manually using `Class.attribute = 1`)
diff --git a/src/runtime/metatype.cs b/src/runtime/metatype.cs
@@ -214,22 +214,30 @@ public static int tp_setattro(IntPtr tp, IntPtr name, IntPtr value)
             if (descr != IntPtr.Zero)
             {
                 IntPtr dt = Runtime.PyObject_TYPE(descr);
-                IntPtr fp = Marshal.ReadIntPtr(dt, TypeOffset.tp_descr_set);
-                if (fp != IntPtr.Zero)
+
+                if (dt == Runtime.PyWrapperDescriptorType
+                    || dt == Runtime.PyMethodType
+                    || typeof(ExtensionType).IsInstanceOfType(GetManagedObject(descr))
+                    )
                 {
-                    return NativeCall.Impl.Int_Call_3(fp, descr, name, value);
+                    IntPtr fp = Marshal.ReadIntPtr(dt, TypeOffset.tp_descr_set);
+                    if (fp != IntPtr.Zero)
+                    {
+                        return NativeCall.Impl.Int_Call_3(fp, descr, name, value);
+                    }
+                    else
+                    {
+                        Exceptions.SetError(Exceptions.AttributeError,
+                            "attribute is read-only");
+                        return -1;
+                    }
                 }
-                Exceptions.SetError(Exceptions.AttributeError,
-                    "attribute is read-only");
-                return -1;
             }
 
-            if (Runtime.PyObject_GenericSetAttr(tp, name, value) < 0)
-            {
-                return -1;
-            }
+            var res = Runtime.PyObject_GenericSetAttr(tp, name, value);
+            Runtime.PyType_Modified(tp);
 
-            return 0;
+            return res;
         }
 
         //====================================================================
@@ -281,7 +289,8 @@ static IntPtr DoInstanceCheck(IntPtr tp, IntPtr args, bool checkType)
         {
             ClassBase cb = GetManagedObject(tp) as ClassBase;
 
-            if (cb == null) {
+            if (cb == null)
+            {
                 Runtime.XIncref(Runtime.PyFalse);
                 return Runtime.PyFalse;
             }
@@ -298,13 +307,15 @@ static IntPtr DoInstanceCheck(IntPtr tp, IntPtr args, bool checkType)
                 else
                     otherType = arg.GetPythonType();
 
-                if (Runtime.PyObject_TYPE(otherType.Handle) != PyCLRMetaType) {
+                if (Runtime.PyObject_TYPE(otherType.Handle) != PyCLRMetaType)
+                {
                     Runtime.XIncref(Runtime.PyFalse);
                     return Runtime.PyFalse;
                 }
 
                 ClassBase otherCb = GetManagedObject(otherType.Handle) as ClassBase;
-                if (otherCb == null) {
+                if (otherCb == null)
+                {
                     Runtime.XIncref(Runtime.PyFalse);
                     return Runtime.PyFalse;
                 }
@@ -323,4 +334,4 @@ public static IntPtr __subclasscheck__(IntPtr tp, IntPtr args)
             return DoInstanceCheck(tp, args, true);
         }
     }
-}
+}
diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs
@@ -254,9 +254,16 @@ internal static void Initialize()
             PyMethodType = Runtime.PyObject_Type(op);
             Runtime.XDecref(op);
 
+            // For some arcane reason, builtins.__dict__.__setitem__ is *not*
+            // a wrapper_descriptor, even though dict.__setitem__ is.
+            //
+            // object.__init__ seems safe, though.
+            op = Runtime.PyObject_GetAttrString(PyBaseObjectType, "__init__");
+            PyWrapperDescriptorType = Runtime.PyObject_Type(op);
+            Runtime.XDecref(op);
+
 #if (PYTHON32 || PYTHON33 || PYTHON34 || PYTHON35)
         Runtime.XDecref(dict);
-        Runtime.XDecref(op);
 #endif
 
             op = Runtime.PyString_FromString("string");
@@ -375,6 +382,7 @@ internal static int AtExit()
         internal static IntPtr PyInstanceType;
         internal static IntPtr PyCLRMetaType;
         internal static IntPtr PyMethodType;
+        internal static IntPtr PyWrapperDescriptorType;
 
         internal static IntPtr PyUnicodeType;
         internal static IntPtr PyStringType;
@@ -404,7 +412,6 @@ internal static int AtExit()
         internal static IntPtr PyNone;
         internal static IntPtr Error;
 
-
         internal static IntPtr GetBoundArgTuple(IntPtr obj, IntPtr args)
         {
             if (Runtime.PyObject_TYPE(args) != Runtime.PyTupleType)
@@ -2129,6 +2136,11 @@ internal static bool PyType_Check(IntPtr ob)
             return PyObject_TypeCheck(ob, Runtime.PyTypeType);
         }
 
+        [DllImport(Runtime.dll, CallingConvention = CallingConvention.Cdecl,
+            ExactSpelling = true, CharSet = CharSet.Ansi)]
+        internal unsafe static extern void
+            PyType_Modified(IntPtr type);
+
         [DllImport(Runtime.dll, CallingConvention = CallingConvention.Cdecl,
             ExactSpelling = true, CharSet = CharSet.Ansi)]
         internal unsafe static extern bool
diff --git a/src/tests/test_class.py b/src/tests/test_class.py
@@ -198,6 +198,17 @@ def __setitem__(self, key, value):
 
         self.assertTrue(table.Count == 3)
 
+    def testAddAndRemoveClassAttribute(self):
+        
+        from System import TimeSpan
+
+        for i in range(100):
+            TimeSpan.new_method = lambda self: self.TotalMinutes
+            ts = TimeSpan.FromHours(1)
+            self.assertTrue(ts.new_method() == 60)
+            del TimeSpan.new_method
+            self.assertFalse(hasattr(ts, "new_method"))
+
 
 class ClassicClass:
     def kind(self):
