Skip to content

Commit 7259b8b

Browse files
fangererfangerer
committed
[GR-80020] Reserve space for native weak reference slots.
PullRequest: graalpython/4896
2 parents 65531b7 + 62e46cc commit 7259b8b

7 files changed

Lines changed: 100 additions & 4 deletions

File tree

‎graalpython/com.oracle.graal.python.cext/src/gcmodule.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,6 +1176,12 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old)
11761176
op = FROM_GC(gc);
11771177
next = GC_NEXT(gc);
11781178

1179+
// GraalPy change: managed objects store weakrefs on the Java side and
1180+
// tagged native stubs have no weakref preheader to inspect here.
1181+
if (is_managed(op)) {
1182+
continue;
1183+
}
1184+
11791185
if (PyWeakref_Check(op)) {
11801186
/* A weakref inside the unreachable set must be cleared. If we
11811187
* allow its callback to execute inside delete_garbage(), it

‎graalpython/com.oracle.graal.python.cext/src/typeobject.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3442,6 +3442,11 @@ subtype_getweakref(PyObject *obj, void *context)
34423442
"This object has no __weakref__");
34433443
return NULL;
34443444
}
3445+
// GraalPy change: tagged stubs have no managed-weakref preheader. Their
3446+
// weakrefs belong to the managed object, not to the native representation.
3447+
if (points_to_py_handle_space(obj)) {
3448+
return GraalPyPrivate_GetManagedWeakRefs(obj);
3449+
}
34453450
_PyObject_ASSERT((PyObject *)type,
34463451
type->tp_weaklistoffset > 0 ||
34473452
type->tp_weaklistoffset == MANAGED_WEAKREF_OFFSET);

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@
3737
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
3838
# SOFTWARE.
3939

40+
import gc
41+
import weakref
42+
from ctypes import c_void_p, sizeof
43+
4044
from . import CPyExtTestCase, CPyExtHeapType, CPyExtType
4145

4246

@@ -120,6 +124,50 @@ class ManagedSubclass(NativeWeakref):
120124
ManagedSubclass.get_native_layout(),
121125
)
122126

127+
def test_weaklistoffset_added_to_native_subclass(self):
128+
NativeLayoutReader = CPyExtType(
129+
'NativeLayoutReader',
130+
'''
131+
static PyObject* get_native_layout(PyObject* unused, PyObject* cls) {
132+
PyTypeObject* type = (PyTypeObject*)cls;
133+
return Py_BuildValue("(nn)", type->tp_basicsize, type->tp_weaklistoffset);
134+
}
135+
''',
136+
tp_methods='{"get_native_layout", (PyCFunction)get_native_layout, METH_O | METH_CLASS, ""}',
137+
)
138+
self.assertEqual(NativeLayoutReader.__weakrefoffset__, 0)
139+
140+
class NativeSubclass(NativeLayoutReader):
141+
pass
142+
143+
class ManagedSubclass(c_void_p):
144+
pass
145+
146+
class SlottedSubclass(ManagedSubclass):
147+
__slots__ = ('extra',)
148+
149+
for cls in (NativeSubclass, c_void_p, ManagedSubclass, SlottedSubclass):
150+
with self.subTest(cls=cls):
151+
weakrefoffset = cls.__weakrefoffset__
152+
self.assertNotEqual(weakrefoffset, 0)
153+
self.assertEqual((cls.__basicsize__, weakrefoffset), NativeLayoutReader.get_native_layout(cls))
154+
if weakrefoffset > 0:
155+
self.assertLessEqual(weakrefoffset + sizeof(c_void_p), cls.__basicsize__)
156+
else:
157+
self.assertEqual(weakrefoffset, -4 * sizeof(c_void_p))
158+
self.assertTrue(cls.__flags__ & (1 << 3)) # Py_TPFLAGS_MANAGED_WEAKREF
159+
160+
self.assertEqual(NativeSubclass.__weakrefoffset__, -4 * sizeof(c_void_p))
161+
self.assertEqual(NativeSubclass.__basicsize__, NativeLayoutReader.__basicsize__)
162+
163+
obj = SlottedSubclass()
164+
obj.extra = obj
165+
ref = weakref.ref(obj)
166+
self.assertIs(ref(), obj)
167+
del obj
168+
gc.collect()
169+
self.assertIsNone(ref())
170+
123171
def test_slots_in_base_and_subclass(self):
124172
N1 = CPyExtHeapType('Nd1', bases=(BaseWithSlots,), cmembers=cmembers(4))
125173

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/WeakRefModuleBuiltins.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import com.oracle.graal.python.builtins.PythonBuiltins;
5959
import com.oracle.graal.python.builtins.modules.weakref.ProxyTypeBuiltins;
6060
import com.oracle.graal.python.builtins.objects.PNone;
61+
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
6162
import com.oracle.graal.python.builtins.objects.dict.PDict;
6263
import com.oracle.graal.python.builtins.objects.module.PythonModule;
6364
import com.oracle.graal.python.builtins.objects.referencetype.PReferenceType;
@@ -243,6 +244,18 @@ public Object getRefs(@SuppressWarnings("unused") Object object) {
243244
}
244245
}
245246

247+
@Builtin(name = "__weakref__", minNumOfPositionalArgs = 1)
248+
@GenerateNodeFactory
249+
public abstract static class GetWeakRefDescriptorNode extends PythonBuiltinNode {
250+
@Specialization
251+
static Object get(PythonAbstractObject object,
252+
@Bind Node inliningTarget,
253+
@Cached HiddenAttr.ReadNode readWeaklistNode) {
254+
Object ref = readWeaklistNode.execute(inliningTarget, object, HiddenAttr.WEAKLIST, null);
255+
return ref == null ? PNone.NONE : ref;
256+
}
257+
}
258+
246259
// _remove_dead_weakref()
247260
@Builtin(name = "_remove_dead_weakref", minNumOfPositionalArgs = 2)
248261
@GenerateNodeFactory

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextWeakrefBuiltins.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiBuiltin;
6060
import com.oracle.graal.python.builtins.modules.weakref.PProxyType;
6161
import com.oracle.graal.python.builtins.objects.PNone;
62+
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
6263
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.ToNativeBorrowedNode;
6364
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
6465
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.NativeToPythonInternalNode;
@@ -67,13 +68,27 @@
6768
import com.oracle.graal.python.builtins.objects.referencetype.PReferenceType;
6869
import com.oracle.graal.python.builtins.objects.referencetype.ReferenceTypeBuiltins.ReferenceTypeNode;
6970
import com.oracle.graal.python.lib.PyObjectCallMethodObjArgs;
71+
import com.oracle.graal.python.nodes.HiddenAttr;
7072
import com.oracle.graal.python.nodes.PRaiseNode;
7173
import com.oracle.graal.python.runtime.PythonContext;
74+
import com.oracle.truffle.api.dsl.Bind;
7275
import com.oracle.truffle.api.dsl.Cached;
7376
import com.oracle.truffle.api.dsl.Specialization;
77+
import com.oracle.truffle.api.nodes.Node;
7478

7579
public final class PythonCextWeakrefBuiltins {
7680

81+
@CApiBuiltin(ret = PyObjectTransfer, args = {PyObject}, call = Direct)
82+
abstract static class GraalPyPrivate_GetManagedWeakRefs extends PythonCextBuiltins.CApiUnaryBuiltinNode {
83+
@Specialization
84+
static Object get(PythonAbstractObject object,
85+
@Bind Node inliningTarget,
86+
@Cached HiddenAttr.ReadNode readWeaklistNode) {
87+
Object ref = readWeaklistNode.execute(inliningTarget, object, HiddenAttr.WEAKLIST, null);
88+
return ref == null ? PNone.NONE : ref;
89+
}
90+
}
91+
7792
@CApiBuiltin(ret = Void, args = {PyObjectRawPointer}, call = Direct, acquireGil = false, canRaise = false)
7893
public static void PyObject_ClearWeakRefs(long pyObject) {
7994
CApiTransitions.removeNativeWeakRef(PythonContext.get(null), pyObject);

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
import static com.oracle.graal.python.builtins.objects.type.TypeFlags.HEAPTYPE;
6565
import static com.oracle.graal.python.builtins.objects.type.TypeFlags.IS_ABSTRACT;
6666
import static com.oracle.graal.python.builtins.objects.type.TypeFlags.MANAGED_DICT;
67+
import static com.oracle.graal.python.builtins.objects.type.TypeFlags.MANAGED_WEAKREF;
6768
import static com.oracle.graal.python.builtins.objects.type.TypeFlags.MATCH_SELF;
6869
import static com.oracle.graal.python.builtins.objects.type.TypeFlags.READY;
6970
import static com.oracle.graal.python.builtins.objects.type.TypeFlags.SUBCLASS_FLAGS;
@@ -100,7 +101,7 @@
100101
import com.oracle.graal.python.builtins.Python3Core;
101102
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
102103
import com.oracle.graal.python.builtins.modules.WarningsModuleBuiltins;
103-
import com.oracle.graal.python.builtins.modules.WeakRefModuleBuiltins.GetWeakRefsNode;
104+
import com.oracle.graal.python.builtins.modules.WeakRefModuleBuiltins.GetWeakRefDescriptorNode;
104105
import com.oracle.graal.python.builtins.modules.WeakRefModuleBuiltinsFactory;
105106
import com.oracle.graal.python.builtins.modules.cext.PythonCextTypeBuiltins;
106107
import com.oracle.graal.python.builtins.objects.PNone;
@@ -255,6 +256,7 @@
255256
public abstract class TypeNodes {
256257

257258
private static final int SIZEOF_PY_OBJECT_PTR = Long.BYTES;
259+
private static final long MANAGED_WEAKREF_OFFSET = -4L * SIZEOF_PY_OBJECT_PTR;
258260

259261
@GenerateUncached
260262
@GenerateInline(false) // footprint reduction 40 -> 21
@@ -2212,9 +2214,9 @@ private static void addDictDescrAttribute(PythonAbstractClass[] basesArray, Pyth
22122214
@TruffleBoundary
22132215
private static void addWeakrefDescrAttribute(PythonClass pythonClass, PythonLanguage language) {
22142216
if (LookupAttributeInMRONode.lookupSlowPath(pythonClass, T___WEAKREF__) == PNone.NO_VALUE) {
2215-
Builtin builtin = GetWeakRefsNode.class.getAnnotation(Builtin.class);
2217+
Builtin builtin = GetWeakRefDescriptorNode.class.getAnnotation(Builtin.class);
22162218
BuiltinFunctionRootNode rootNode = PythonLanguage.get(null).createCachedRootNode(
2217-
l -> new BuiltinFunctionRootNode(l, builtin, WeakRefModuleBuiltinsFactory.GetWeakRefsNodeFactory.getInstance(), true), GetWeakRefsNode.class);
2219+
l -> new BuiltinFunctionRootNode(l, builtin, WeakRefModuleBuiltinsFactory.GetWeakRefDescriptorNodeFactory.getInstance(), true), GetWeakRefDescriptorNode.class);
22182220
setAttribute(T___WEAKREF__, builtin, rootNode, pythonClass, language);
22192221
}
22202222
}
@@ -2284,7 +2286,12 @@ private static void addNativeSlots(TypeNewContext ctx, PythonManagedClass python
22842286
dictOffset = -1;
22852287
}
22862288
if (ctx.addWeak) {
2287-
weakListOffset = slotOffset;
2289+
// The weakref pointer lives in the preheader, not after the instance slots.
2290+
weakListOffset = MANAGED_WEAKREF_OFFSET;
2291+
}
2292+
if (weakListOffset == MANAGED_WEAKREF_OFFSET) {
2293+
long flags = GetTypeFlagsNode.executeUncached(pythonClass);
2294+
SetTypeFlagsNode.executeUncached(pythonClass, flags | MANAGED_WEAKREF);
22882295
}
22892296

22902297
SetDictOffsetNode.executeUncached(pythonClass, dictOffset);

‎graalpython/lib-python/3/test/test_capi/test_misc.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,8 @@ class C4(_testcapi.HeapCTypeWithManagedWeakref, list):
771771
inst = C4()
772772
inst.append(0)
773773
str(inst.__weakref__)
774+
ref = weakref.ref(inst)
775+
self.assertIs(inst.__weakref__, ref)
774776

775777
for cls in (_testcapi.HeapCTypeWithManagedDict, _testcapi.HeapCTypeWithManagedWeakref):
776778
for cls2 in (_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithWeakref):

0 commit comments

Comments
 (0)