Skip to content

Commit b039946

Browse files
steve-ssteve-s
committed
[GR-71869] Fix memory leaks in LookupAttributeInMRONode.lookupConstantMROCached and super builtins.
PullRequest: graalpython/4220
2 parents a8eada3 + 13ceeda commit b039946

7 files changed

Lines changed: 27 additions & 16 deletions

File tree

‎graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_functools.txt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ test.test_functools.TestLRUC.test_lru_cache_threaded @ darwin-arm64,linux-aarch6
3636
test.test_functools.TestLRUC.test_lru_cache_threaded2 @ darwin-arm64,linux-aarch64,linux-aarch64-github,linux-x86_64,linux-x86_64-github,win32-AMD64,win32-AMD64-github
3737
test.test_functools.TestLRUC.test_lru_cache_threaded3 @ darwin-arm64,linux-aarch64,linux-aarch64-github,linux-x86_64,linux-x86_64-github,win32-AMD64,win32-AMD64-github
3838
test.test_functools.TestLRUC.test_lru_cache_typed_is_not_recursive @ darwin-arm64,linux-aarch64,linux-aarch64-github,linux-x86_64,linux-x86_64-github,win32-AMD64,win32-AMD64-github
39-
# GR-71869
40-
!test.test_functools.TestLRUC.test_lru_cache_weakrefable @ darwin-arm64,linux-x86_64,win32-AMD64
39+
test.test_functools.TestLRUC.test_lru_cache_weakrefable @ darwin-arm64,linux-x86_64,win32-AMD64
4140
test.test_functools.TestLRUC.test_lru_hash_only_once @ darwin-arm64,linux-aarch64,linux-aarch64-github,linux-x86_64,linux-x86_64-github,win32-AMD64,win32-AMD64-github
4241
test.test_functools.TestLRUC.test_lru_method @ darwin-arm64,linux-aarch64,linux-aarch64-github,linux-x86_64,linux-x86_64-github,win32-AMD64,win32-AMD64-github
4342
test.test_functools.TestLRUC.test_lru_no_args @ darwin-arm64,linux-aarch64,linux-aarch64-github,linux-x86_64,linux-x86_64-github,win32-AMD64,win32-AMD64-github

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/superobject/SuperBuiltins.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ final Object executeCached(SuperObject self) {
148148

149149
@Specialization(guards = {"isSingleContext()", "self == cachedSelf"}, assumptions = {"cachedSelf.getNeverReinitializedAssumption()"}, limit = "1")
150150
static Object cached(@NeverDefault @SuppressWarnings("unused") SuperObject self,
151-
@SuppressWarnings("unused") @Cached("self") SuperObject cachedSelf,
152-
@Cached(value = "self.getType()") Object type) {
151+
@SuppressWarnings("unused") @Cached(value = "self", weak = true) SuperObject cachedSelf,
152+
@Cached(value = "self.getType()", weak = true) Object type) {
153153
return type;
154154
}
155155

@@ -167,8 +167,8 @@ abstract static class GetObjectTypeNode extends PNodeWithContext {
167167

168168
@Specialization(guards = {"isSingleContext()", "self == cachedSelf"}, assumptions = {"cachedSelf.getNeverReinitializedAssumption()"}, limit = "1")
169169
static Object cached(@NeverDefault @SuppressWarnings("unused") SuperObject self,
170-
@SuppressWarnings("unused") @Cached("self") SuperObject cachedSelf,
171-
@Cached(value = "self.getObjectType()") Object type) {
170+
@SuppressWarnings("unused") @Cached(value = "self", weak = true) SuperObject cachedSelf,
171+
@Cached(value = "self.getObjectType()", weak = true) Object type) {
172172
return type;
173173
}
174174

@@ -190,8 +190,8 @@ final Object executeCached(SuperObject self) {
190190

191191
@Specialization(guards = {"isSingleContext()", "self == cachedSelf"}, assumptions = {"cachedSelf.getNeverReinitializedAssumption()"}, limit = "1")
192192
static Object cached(@NeverDefault @SuppressWarnings("unused") SuperObject self,
193-
@SuppressWarnings("unused") @Cached("self") SuperObject cachedSelf,
194-
@Cached(value = "self.getObject()") Object object) {
193+
@SuppressWarnings("unused") @Cached(value = "self", weak = true) SuperObject cachedSelf,
194+
@Cached(value = "self.getObject()", weak = true) Object object) {
195195
return object;
196196
}
197197

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/LookupAttributeInMRONode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ MroSequenceStorage.FinalAttributeAssumptionPair findAttrAndAssumptionInMRO(Objec
248248
assumptions = "cachedAttrInMROInfo.getAssumption()")
249249
static Object lookupConstantMROCached(Object klass,
250250
@Bind Node inliningTarget,
251-
@Cached("klass") Object cachedKlass,
251+
@Cached(value = "klass", weak = true) Object cachedKlass,
252252
@Cached IsSameTypeNode isSameTypeNode,
253253
@Cached("findAttrAndAssumptionInMRO(cachedKlass)") MroSequenceStorage.FinalAttributeAssumptionPair cachedAttrInMROInfo) {
254254
return cachedAttrInMROInfo.getValue();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/ReadAttributeFromPythonObjectNode.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -45,6 +45,7 @@
4545
import com.oracle.graal.python.builtins.objects.object.PythonObject;
4646
import com.oracle.graal.python.builtins.objects.type.PythonManagedClass;
4747
import com.oracle.graal.python.nodes.PNodeWithContext;
48+
import com.oracle.graal.python.util.PythonUtils;
4849
import com.oracle.truffle.api.dsl.Cached;
4950
import com.oracle.truffle.api.dsl.GenerateInline;
5051
import com.oracle.truffle.api.dsl.GenerateUncached;
@@ -99,7 +100,7 @@ protected static boolean isLongLivedObject(DynamicObject object) {
99100

100101
@Idempotent
101102
protected static boolean isPrimitive(Object value) {
102-
return value instanceof Integer || value instanceof Long || value instanceof Boolean || value instanceof Double;
103+
return PythonUtils.isPrimitive(value);
103104
}
104105

105106
@NonIdempotent

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/sequence/storage/MroSequenceStorage.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -54,6 +54,7 @@
5454
import com.oracle.truffle.api.dsl.NonIdempotent;
5555
import com.oracle.truffle.api.strings.TruffleString;
5656
import com.oracle.truffle.api.utilities.CyclicAssumption;
57+
import com.oracle.truffle.api.utilities.TruffleWeakReference;
5758

5859
public final class MroSequenceStorage extends ArrayBasedSequenceStorage {
5960

@@ -71,7 +72,8 @@ public final class MroSequenceStorage extends ArrayBasedSequenceStorage {
7172

7273
public static final class FinalAttributeAssumptionPair {
7374
@CompilationFinal private Assumption assumption;
74-
@CompilationFinal private Object value;
75+
@CompilationFinal private TruffleWeakReference<Object> value;
76+
private Object valueStrongReference;
7577

7678
public FinalAttributeAssumptionPair() {
7779
this.assumption = Truffle.getRuntime().createAssumption("attribute in MRO final");
@@ -87,11 +89,16 @@ public void invalidate() {
8789

8890
@NonIdempotent
8991
public Object getValue() {
90-
return value;
92+
return value.get();
9193
}
9294

9395
public void setValue(Object value) {
94-
this.value = value;
96+
if (PythonUtils.isPrimitive(value)) {
97+
// DynamicObjectStorage does not store the boxed object, but the raw primitive
98+
// value, so we must protect the boxed object from GC ourselves
99+
valueStrongReference = value;
100+
}
101+
this.value = new TruffleWeakReference<>(value);
95102
}
96103

97104
@NonIdempotent

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/util/PythonUtils.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,10 @@ public static Object builtinClassToType(Object cls) {
838838
return cls;
839839
}
840840

841+
public static boolean isPrimitive(Object value) {
842+
return value instanceof Integer || value instanceof Long || value instanceof Boolean || value instanceof Double;
843+
}
844+
841845
public static final class NodeCounterWithLimit implements NodeVisitor {
842846
private int count;
843847
private final int limit;

‎graalpython/lib-python/3/test/test_functools.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1872,7 +1872,7 @@ def test_staticmethod(x):
18721872

18731873
del A
18741874
del test_function
1875-
gc.collect()
1875+
support.gc_collect()
18761876

18771877
for ref in refs:
18781878
self.assertIsNone(ref())

0 commit comments

Comments
 (0)