diff --git a/graalpython/com.oracle.graal.python.cext/src/gcmodule.c b/graalpython/com.oracle.graal.python.cext/src/gcmodule.c index cceda1bcab..42a1f7496b 100644 --- a/graalpython/com.oracle.graal.python.cext/src/gcmodule.c +++ b/graalpython/com.oracle.graal.python.cext/src/gcmodule.c @@ -1176,6 +1176,12 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old) op = FROM_GC(gc); next = GC_NEXT(gc); + // GraalPy change: managed objects store weakrefs on the Java side and + // tagged native stubs have no weakref preheader to inspect here. + if (is_managed(op)) { + continue; + } + if (PyWeakref_Check(op)) { /* A weakref inside the unreachable set must be cleared. If we * allow its callback to execute inside delete_garbage(), it diff --git a/graalpython/com.oracle.graal.python.cext/src/typeobject.c b/graalpython/com.oracle.graal.python.cext/src/typeobject.c index 5013059fe7..5816f24d16 100644 --- a/graalpython/com.oracle.graal.python.cext/src/typeobject.c +++ b/graalpython/com.oracle.graal.python.cext/src/typeobject.c @@ -3442,6 +3442,11 @@ subtype_getweakref(PyObject *obj, void *context) "This object has no __weakref__"); return NULL; } + // GraalPy change: tagged stubs have no managed-weakref preheader. Their + // weakrefs belong to the managed object, not to the native representation. + if (points_to_py_handle_space(obj)) { + return GraalPyPrivate_GetManagedWeakRefs(obj); + } _PyObject_ASSERT((PyObject *)type, type->tp_weaklistoffset > 0 || type->tp_weaklistoffset == MANAGED_WEAKREF_OFFSET); diff --git a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_indexed_slots.py b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_indexed_slots.py index a1bf461207..581996c1d7 100644 --- a/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_indexed_slots.py +++ b/graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_indexed_slots.py @@ -37,6 +37,10 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +import gc +import weakref +from ctypes import c_void_p, sizeof + from . import CPyExtTestCase, CPyExtHeapType, CPyExtType @@ -120,6 +124,50 @@ class ManagedSubclass(NativeWeakref): ManagedSubclass.get_native_layout(), ) + def test_weaklistoffset_added_to_native_subclass(self): + NativeLayoutReader = CPyExtType( + 'NativeLayoutReader', + ''' + static PyObject* get_native_layout(PyObject* unused, PyObject* cls) { + PyTypeObject* type = (PyTypeObject*)cls; + return Py_BuildValue("(nn)", type->tp_basicsize, type->tp_weaklistoffset); + } + ''', + tp_methods='{"get_native_layout", (PyCFunction)get_native_layout, METH_O | METH_CLASS, ""}', + ) + self.assertEqual(NativeLayoutReader.__weakrefoffset__, 0) + + class NativeSubclass(NativeLayoutReader): + pass + + class ManagedSubclass(c_void_p): + pass + + class SlottedSubclass(ManagedSubclass): + __slots__ = ('extra',) + + for cls in (NativeSubclass, c_void_p, ManagedSubclass, SlottedSubclass): + with self.subTest(cls=cls): + weakrefoffset = cls.__weakrefoffset__ + self.assertNotEqual(weakrefoffset, 0) + self.assertEqual((cls.__basicsize__, weakrefoffset), NativeLayoutReader.get_native_layout(cls)) + if weakrefoffset > 0: + self.assertLessEqual(weakrefoffset + sizeof(c_void_p), cls.__basicsize__) + else: + self.assertEqual(weakrefoffset, -4 * sizeof(c_void_p)) + self.assertTrue(cls.__flags__ & (1 << 3)) # Py_TPFLAGS_MANAGED_WEAKREF + + self.assertEqual(NativeSubclass.__weakrefoffset__, -4 * sizeof(c_void_p)) + self.assertEqual(NativeSubclass.__basicsize__, NativeLayoutReader.__basicsize__) + + obj = SlottedSubclass() + obj.extra = obj + ref = weakref.ref(obj) + self.assertIs(ref(), obj) + del obj + gc.collect() + self.assertIsNone(ref()) + def test_slots_in_base_and_subclass(self): N1 = CPyExtHeapType('Nd1', bases=(BaseWithSlots,), cmembers=cmembers(4)) diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/WeakRefModuleBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/WeakRefModuleBuiltins.java index 715d4c95b8..1b432d0974 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/WeakRefModuleBuiltins.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/WeakRefModuleBuiltins.java @@ -58,6 +58,7 @@ import com.oracle.graal.python.builtins.PythonBuiltins; import com.oracle.graal.python.builtins.modules.weakref.ProxyTypeBuiltins; import com.oracle.graal.python.builtins.objects.PNone; +import com.oracle.graal.python.builtins.objects.PythonAbstractObject; import com.oracle.graal.python.builtins.objects.dict.PDict; import com.oracle.graal.python.builtins.objects.module.PythonModule; import com.oracle.graal.python.builtins.objects.referencetype.PReferenceType; @@ -243,6 +244,18 @@ public Object getRefs(@SuppressWarnings("unused") Object object) { } } + @Builtin(name = "__weakref__", minNumOfPositionalArgs = 1) + @GenerateNodeFactory + public abstract static class GetWeakRefDescriptorNode extends PythonBuiltinNode { + @Specialization + static Object get(PythonAbstractObject object, + @Bind Node inliningTarget, + @Cached HiddenAttr.ReadNode readWeaklistNode) { + Object ref = readWeaklistNode.execute(inliningTarget, object, HiddenAttr.WEAKLIST, null); + return ref == null ? PNone.NONE : ref; + } + } + // _remove_dead_weakref() @Builtin(name = "_remove_dead_weakref", minNumOfPositionalArgs = 2) @GenerateNodeFactory diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextWeakrefBuiltins.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextWeakrefBuiltins.java index d1977a2385..e3acbf3747 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextWeakrefBuiltins.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextWeakrefBuiltins.java @@ -59,6 +59,7 @@ import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiBuiltin; import com.oracle.graal.python.builtins.modules.weakref.PProxyType; import com.oracle.graal.python.builtins.objects.PNone; +import com.oracle.graal.python.builtins.objects.PythonAbstractObject; import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.ToNativeBorrowedNode; import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions; import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.NativeToPythonInternalNode; @@ -67,13 +68,27 @@ import com.oracle.graal.python.builtins.objects.referencetype.PReferenceType; import com.oracle.graal.python.builtins.objects.referencetype.ReferenceTypeBuiltins.ReferenceTypeNode; import com.oracle.graal.python.lib.PyObjectCallMethodObjArgs; +import com.oracle.graal.python.nodes.HiddenAttr; import com.oracle.graal.python.nodes.PRaiseNode; import com.oracle.graal.python.runtime.PythonContext; +import com.oracle.truffle.api.dsl.Bind; import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.nodes.Node; public final class PythonCextWeakrefBuiltins { + @CApiBuiltin(ret = PyObjectTransfer, args = {PyObject}, call = Direct) + abstract static class GraalPyPrivate_GetManagedWeakRefs extends PythonCextBuiltins.CApiUnaryBuiltinNode { + @Specialization + static Object get(PythonAbstractObject object, + @Bind Node inliningTarget, + @Cached HiddenAttr.ReadNode readWeaklistNode) { + Object ref = readWeaklistNode.execute(inliningTarget, object, HiddenAttr.WEAKLIST, null); + return ref == null ? PNone.NONE : ref; + } + } + @CApiBuiltin(ret = Void, args = {PyObjectRawPointer}, call = Direct, acquireGil = false, canRaise = false) public static void PyObject_ClearWeakRefs(long pyObject) { CApiTransitions.removeNativeWeakRef(PythonContext.get(null), pyObject); diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java index a6890a2d5d..d187c24935 100644 --- a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java +++ b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeNodes.java @@ -64,6 +64,7 @@ import static com.oracle.graal.python.builtins.objects.type.TypeFlags.HEAPTYPE; import static com.oracle.graal.python.builtins.objects.type.TypeFlags.IS_ABSTRACT; import static com.oracle.graal.python.builtins.objects.type.TypeFlags.MANAGED_DICT; +import static com.oracle.graal.python.builtins.objects.type.TypeFlags.MANAGED_WEAKREF; import static com.oracle.graal.python.builtins.objects.type.TypeFlags.MATCH_SELF; import static com.oracle.graal.python.builtins.objects.type.TypeFlags.READY; import static com.oracle.graal.python.builtins.objects.type.TypeFlags.SUBCLASS_FLAGS; @@ -100,7 +101,7 @@ import com.oracle.graal.python.builtins.Python3Core; import com.oracle.graal.python.builtins.PythonBuiltinClassType; import com.oracle.graal.python.builtins.modules.WarningsModuleBuiltins; -import com.oracle.graal.python.builtins.modules.WeakRefModuleBuiltins.GetWeakRefsNode; +import com.oracle.graal.python.builtins.modules.WeakRefModuleBuiltins.GetWeakRefDescriptorNode; import com.oracle.graal.python.builtins.modules.WeakRefModuleBuiltinsFactory; import com.oracle.graal.python.builtins.modules.cext.PythonCextTypeBuiltins; import com.oracle.graal.python.builtins.objects.PNone; @@ -255,6 +256,7 @@ public abstract class TypeNodes { private static final int SIZEOF_PY_OBJECT_PTR = Long.BYTES; + private static final long MANAGED_WEAKREF_OFFSET = -4L * SIZEOF_PY_OBJECT_PTR; @GenerateUncached @GenerateInline(false) // footprint reduction 40 -> 21 @@ -2212,9 +2214,9 @@ private static void addDictDescrAttribute(PythonAbstractClass[] basesArray, Pyth @TruffleBoundary private static void addWeakrefDescrAttribute(PythonClass pythonClass, PythonLanguage language) { if (LookupAttributeInMRONode.lookupSlowPath(pythonClass, T___WEAKREF__) == PNone.NO_VALUE) { - Builtin builtin = GetWeakRefsNode.class.getAnnotation(Builtin.class); + Builtin builtin = GetWeakRefDescriptorNode.class.getAnnotation(Builtin.class); BuiltinFunctionRootNode rootNode = PythonLanguage.get(null).createCachedRootNode( - l -> new BuiltinFunctionRootNode(l, builtin, WeakRefModuleBuiltinsFactory.GetWeakRefsNodeFactory.getInstance(), true), GetWeakRefsNode.class); + l -> new BuiltinFunctionRootNode(l, builtin, WeakRefModuleBuiltinsFactory.GetWeakRefDescriptorNodeFactory.getInstance(), true), GetWeakRefDescriptorNode.class); setAttribute(T___WEAKREF__, builtin, rootNode, pythonClass, language); } } @@ -2284,7 +2286,12 @@ private static void addNativeSlots(TypeNewContext ctx, PythonManagedClass python dictOffset = -1; } if (ctx.addWeak) { - weakListOffset = slotOffset; + // The weakref pointer lives in the preheader, not after the instance slots. + weakListOffset = MANAGED_WEAKREF_OFFSET; + } + if (weakListOffset == MANAGED_WEAKREF_OFFSET) { + long flags = GetTypeFlagsNode.executeUncached(pythonClass); + SetTypeFlagsNode.executeUncached(pythonClass, flags | MANAGED_WEAKREF); } SetDictOffsetNode.executeUncached(pythonClass, dictOffset); diff --git a/graalpython/lib-python/3/test/test_capi/test_misc.py b/graalpython/lib-python/3/test/test_capi/test_misc.py index 9d5a7c2caa..646a3a1b35 100644 --- a/graalpython/lib-python/3/test/test_capi/test_misc.py +++ b/graalpython/lib-python/3/test/test_capi/test_misc.py @@ -771,6 +771,8 @@ class C4(_testcapi.HeapCTypeWithManagedWeakref, list): inst = C4() inst.append(0) str(inst.__weakref__) + ref = weakref.ref(inst) + self.assertIs(inst.__weakref__, ref) for cls in (_testcapi.HeapCTypeWithManagedDict, _testcapi.HeapCTypeWithManagedWeakref): for cls2 in (_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithWeakref):