Cache native slot wrappers + test of tp_setattr vs tp_setattro inheritance

Author: steve-s

graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_tp_slots.py

@@ -47,13 +47,21 @@
                          static PyObject* get_tp_attro(PyObject* unused, PyObject* object) {
                              return PyLong_FromVoidPtr(Py_TYPE(object)->tp_getattro);
                          }
+                         static PyObject* get_tp_setattr(PyObject* unused, PyObject* object) {
+                             return PyLong_FromVoidPtr(Py_TYPE(object)->tp_setattr);
+                         }
+                         static PyObject* get_tp_setattro(PyObject* unused, PyObject* object) {
+                             return PyLong_FromVoidPtr(Py_TYPE(object)->tp_setattro);
+                         }
                          static PyObject* get_tp_descr_get(PyObject* unused, PyObject* object) {
                              return PyLong_FromVoidPtr(Py_TYPE(object)->tp_descr_get);
                          }
                          """,
                          tp_methods=
                          '{"get_tp_attr", (PyCFunction)get_tp_attr, METH_O | METH_STATIC, ""},' +
                          '{"get_tp_attro", (PyCFunction)get_tp_attro, METH_O | METH_STATIC, ""},' +
+                         '{"get_tp_setattr", (PyCFunction)get_tp_setattr, METH_O | METH_STATIC, ""},' +
+                         '{"get_tp_setattro", (PyCFunction)get_tp_setattro, METH_O | METH_STATIC, ""},' +
                          '{"get_tp_descr_get", (PyCFunction)get_tp_descr_get, METH_O | METH_STATIC, ""}')
 
 
@@ -205,6 +213,45 @@ def test_setattr_wrapper():
     assert x.__setattr__("bar", 42) is None
 
 
+def test_setattr_vs_setattro_inheritance():
+    TestSetAttrOInheritance = CPyExtType("TestSetAttrOInheritance",
+                                         '''
+                                         int testattr_set(PyObject* self, char* key, PyObject* value) {
+                                             Py_XDECREF(((TestSetAttrOInheritanceObject*)self)->payload);
+                                             if (value != NULL) {
+                                                 Py_INCREF(value);
+                                             }
+                                             ((TestSetAttrOInheritanceObject*)self)->payload = value;
+                                             return 0;
+                                         }
+
+                                         PyObject* get_payload(PyObject *self) {
+                                              PyObject* r = ((TestSetAttrOInheritanceObject*)self)->payload;
+                                              if (r == NULL) Py_RETURN_NONE;
+                                              Py_INCREF(r);
+                                              return r;
+                                         }
+                                         ''',
+                                         cmembers='PyObject* payload;',
+                                         tp_methods='{"get_payload", (PyCFunction)get_payload, METH_NOARGS, ""}',
+                                         tp_setattr="testattr_set")
+
+    x = TestSetAttrOInheritance()
+    x.foo = 42
+    assert x.get_payload() == 42
+
+    class Managed(TestSetAttrOInheritance):
+        pass
+
+    x = Managed()
+    x.foo = 42  # calls slot_tp_setattro, which calls __setattr__, which wraps the object.tp_setattro
+    assert x.get_payload() is None
+
+    assert SlotsGetter.get_tp_setattro(object()) == SlotsGetter.get_tp_setattro(Managed())
+    assert SlotsGetter.get_tp_setattro(TestSetAttrOInheritance()) == 0
+    assert SlotsGetter.get_tp_setattr(TestSetAttrOInheritance()) != 0
+
+
 def test_sq_ass_item_wrapper():
     TestSqAssItemWrapperReturn = CPyExtType("TestSqAssItemWrapperReturn",
                                           "static int myassitem(PyObject *self, Py_ssize_t i, PyObject *v) { return 0; }",

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TpSlots.java

@@ -206,18 +206,18 @@ private interface TpSlotGetter {
      */
     @FunctionalInterface
     private interface NativeWrapperFactory {
-        PyProcsWrapper create(TpSlotManaged slot);
+        TpSlotWrapper create(TpSlotManaged slot);
 
         final class Unimplemented implements NativeWrapperFactory {
             @Override
-            public PyProcsWrapper create(TpSlotManaged slot) {
+            public TpSlotWrapper create(TpSlotManaged slot) {
                 throw new IllegalStateException("TODO: " + slot);
             }
         }
 
         record ShouldNotReach(String slotName) implements NativeWrapperFactory {
             @Override
-            public PyProcsWrapper create(TpSlotManaged slot) {
+            public TpSlotWrapper create(TpSlotManaged slot) {
                 throw new IllegalStateException(String.format("Slot %s should never be assigned a managed slot value.", slotName));
             }
         }
@@ -406,7 +406,7 @@ public Object readFromNative(PythonAbstractNativeObject pythonClass) {
             return field;
         }
 
-        public PyProcsWrapper createNativeWrapper(TpSlotManaged slot) {
+        public TpSlotWrapper createNativeWrapper(TpSlotManaged slot) {
             return nativeWrapperFactory.create(slot);
         }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/slots/TpSlot.java

@@ -88,7 +88,11 @@ public static Object toNative(TpSlotMeta slotMeta, TpSlot slot, Object defaultVa
             // This returns PyProcsWrapper, which will, in its toNative message, register the
             // pointer in C API context, such that we can map back from a pointer that we get from C
             // to the PyProcsWrapper and from that to the slot instance again in TpSlots#fromNative
-            return slotMeta.createNativeWrapper(managedSlot);
+            assert PythonContext.get(null).ownsGil(); // without GIL: use AtomicReference & CAS
+            if (managedSlot.slotWrapper == null) {
+                managedSlot.slotWrapper = slotMeta.createNativeWrapper(managedSlot);
+            }
+            return managedSlot.slotWrapper;
         } else {
             throw CompilerDirectives.shouldNotReachHere("TpSlotWrapper should wrap only managed slots. Native slots should go directly to native unwrapped.");
         }
@@ -121,6 +125,7 @@ public static TpSlot fromNative(PythonContext ctx, Object ptr, InteropLibrary in
      * Marker base class for managed slots: either builtin slots or user defined Python slots.
      */
     public abstract static sealed class TpSlotManaged extends TpSlot permits TpSlotBuiltin, TpSlotPython {
+        private TpSlotWrapper slotWrapper;
     }
 
     /**