Skip to content

Commit e0f2f07

Browse files
msimacekmsimacek
committed
[GR-53037][GR-52848] Fix leak when deleting from native storage
PullRequest: graalpython/3275
2 parents 4a5efed + 42b7bc3 commit e0f2f07

19 files changed

+169
-273
lines changed

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -529,14 +529,6 @@ PyAPI_FUNC(void) PyTruffle_ObjectArrayRelease(PyObject** array, int32_t size) {
529529
}
530530
}
531531

532-
PyAPI_FUNC(void) PyTruffle_SetStorageItem(PyObject** ptr, int32_t index, PyObject* newitem) {
533-
Py_XSETREF(ptr[index], newitem);
534-
}
535-
536-
PyAPI_FUNC(void) PyTruffle_InitializeStorageItem(PyObject** ptr, int32_t index, PyObject* newitem) {
537-
ptr[index] = newitem;
538-
}
539-
540532
#define ReadMember(object, offset, T) ((T*)(((char*)object) + offset))[0]
541533

542534
PyAPI_FUNC(int) ReadShortMember(void* object, Py_ssize_t offset) {

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

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,10 @@
116116
import com.oracle.graal.python.builtins.objects.buffer.PythonBufferAccessLibrary;
117117
import com.oracle.graal.python.builtins.objects.bytes.BytesNodes.ToBytesNode;
118118
import com.oracle.graal.python.builtins.objects.bytes.PBytes;
119-
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
120119
import com.oracle.graal.python.builtins.objects.cext.capi.CApiContext;
121-
import com.oracle.graal.python.builtins.objects.cext.capi.CApiGuards;
122-
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.AddRefCntNode;
123-
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PCallCapiFunction;
120+
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes;
124121
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
122+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
125123
import com.oracle.graal.python.builtins.objects.cext.common.CArrayWrappers.CByteArrayWrapper;
126124
import com.oracle.graal.python.builtins.objects.cext.common.LoadCExtException.ApiInitException;
127125
import com.oracle.graal.python.builtins.objects.cext.common.LoadCExtException.ImportException;
@@ -134,7 +132,6 @@
134132
import com.oracle.graal.python.builtins.objects.dict.PDict;
135133
import com.oracle.graal.python.builtins.objects.function.PKeyword;
136134
import com.oracle.graal.python.builtins.objects.module.PythonModule;
137-
import com.oracle.graal.python.builtins.objects.object.PythonObject;
138135
import com.oracle.graal.python.builtins.objects.str.StringBuiltins.PrefixSuffixNode;
139136
import com.oracle.graal.python.builtins.objects.str.StringUtils.SimpleTruffleStringFormatNode;
140137
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
@@ -1696,55 +1693,29 @@ static Object doit(Object obj,
16961693
}
16971694
}
16981695

1699-
@ImportStatic(CApiGuards.class)
17001696
@Builtin(name = "Py_INCREF", minNumOfPositionalArgs = 1)
17011697
@GenerateNodeFactory
17021698
protected abstract static class PyINCREFNode extends PythonUnaryBuiltinNode {
17031699

17041700
@Specialization
17051701
@TruffleBoundary
17061702
static Object doGeneric(Object arg) {
1707-
1708-
Object pointerObject = null;
1709-
if (arg instanceof PythonAbstractNativeObject nativeObject) {
1710-
pointerObject = nativeObject.getPtr();
1711-
} else if (arg instanceof PythonObject managedObject) {
1712-
pointerObject = managedObject.getNativeWrapper();
1713-
}
1714-
// ignore other cases; also: don't allocate wrappers just because of this
1715-
1716-
if (pointerObject != null) {
1717-
AddRefCntNode.executeUncached(pointerObject, //
1718-
1 /* that's what this function is for */ + //
1719-
1 /* that for returning it */);
1720-
}
1703+
CApiContext.ensureCapiWasLoaded();
1704+
CApiTransitions.PythonToNativeNewRefNode.executeUncached(arg);
17211705
return arg;
17221706
}
17231707
}
17241708

1725-
@ImportStatic(CApiGuards.class)
17261709
@Builtin(name = "Py_DECREF", minNumOfPositionalArgs = 1)
17271710
@GenerateNodeFactory
17281711
protected abstract static class PyDECREFNode extends PythonUnaryBuiltinNode {
17291712

17301713
@Specialization
17311714
@TruffleBoundary
17321715
static Object doGeneric(Object arg) {
1733-
1734-
Object pointerObject = null;
1735-
if (arg instanceof PythonAbstractNativeObject nativeObject) {
1736-
pointerObject = nativeObject.getPtr();
1737-
} else if (arg instanceof PythonObject managedObject) {
1738-
pointerObject = managedObject.getNativeWrapper();
1739-
}
1740-
// ignore other cases; also: don't allocate wrappers just because of this
1741-
1742-
if (pointerObject != null) {
1743-
// that's what this function is for
1744-
PCallCapiFunction.callUncached(NativeCAPISymbol.FUN_DECREF, pointerObject);
1745-
// that for returning it
1746-
PCallCapiFunction.callUncached(NativeCAPISymbol.FUN_INCREF, pointerObject);
1747-
}
1716+
CApiContext.ensureCapiWasLoaded();
1717+
Object nativePointer = CApiTransitions.PythonToNativeNode.executeUncached(arg);
1718+
CExtNodes.DecRefPointerNode.executeUncached(nativePointer);
17481719
return arg;
17491720
}
17501721
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CExtNodes.java

Lines changed: 48 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,11 @@
4343
import static com.oracle.graal.python.builtins.objects.PNone.NO_VALUE;
4444
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PTR_ADD;
4545
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PTR_COMPARE;
46+
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_DEALLOC;
4647
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_OBJECT_GC_DEL;
4748
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_TRUFFLE_MEMORYVIEW_FROM_OBJECT;
4849
import static com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol.FUN_PY_TYPE_GENERIC_ALLOC;
50+
import static com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper.IMMORTAL_REFCNT;
4951
import static com.oracle.graal.python.builtins.objects.cext.structs.CConstants.PYLONG_BITS_IN_DIGIT;
5052
import static com.oracle.graal.python.builtins.objects.cext.structs.CFields.PyFloatObject__ob_fval;
5153
import static com.oracle.graal.python.builtins.objects.cext.structs.CFields.PyMethodDef__ml_doc;
@@ -58,6 +60,7 @@
5860
import static com.oracle.graal.python.builtins.objects.cext.structs.CFields.PyModuleDef__m_methods;
5961
import static com.oracle.graal.python.builtins.objects.cext.structs.CFields.PyModuleDef__m_size;
6062
import static com.oracle.graal.python.builtins.objects.cext.structs.CFields.PyModuleDef__m_slots;
63+
import static com.oracle.graal.python.builtins.objects.cext.structs.CFields.PyObject__ob_refcnt;
6164
import static com.oracle.graal.python.builtins.objects.cext.structs.CFields.PyObject__ob_type;
6265
import static com.oracle.graal.python.builtins.objects.cext.structs.CFields.PyTypeObject__tp_as_buffer;
6366
import static com.oracle.graal.python.nodes.HiddenAttr.METHOD_DEF_PTR;
@@ -83,7 +86,6 @@
8386
import com.oracle.graal.python.builtins.objects.cext.PythonNativeClass;
8487
import com.oracle.graal.python.builtins.objects.cext.PythonNativeObject;
8588
import com.oracle.graal.python.builtins.objects.cext.PythonNativeVoidPtr;
86-
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.AddRefCntNodeGen;
8789
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.AsCharPointerNodeGen;
8890
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.CreateFunctionNodeGen;
8991
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.FromCharPointerNodeGen;
@@ -113,6 +115,7 @@
113115
import com.oracle.graal.python.builtins.objects.cext.common.CExtContext;
114116
import com.oracle.graal.python.builtins.objects.cext.common.CExtContext.ModuleSpec;
115117
import com.oracle.graal.python.builtins.objects.cext.common.GetNextVaArgNode;
118+
import com.oracle.graal.python.builtins.objects.cext.common.NativePointer;
116119
import com.oracle.graal.python.builtins.objects.cext.structs.CFields;
117120
import com.oracle.graal.python.builtins.objects.cext.structs.CStructAccess;
118121
import com.oracle.graal.python.builtins.objects.cext.structs.CStructs;
@@ -200,6 +203,7 @@
200203
import com.oracle.truffle.api.interop.UnsupportedTypeException;
201204
import com.oracle.truffle.api.library.CachedLibrary;
202205
import com.oracle.truffle.api.nodes.Node;
206+
import com.oracle.truffle.api.profiles.InlinedBranchProfile;
203207
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
204208
import com.oracle.truffle.api.source.Source;
205209
import com.oracle.truffle.api.strings.TruffleString;
@@ -1132,51 +1136,56 @@ static PRaiseNativeNode doIt(@Cached(inline = false) PRaiseNativeNode node) {
11321136
@GenerateInline
11331137
@GenerateCached(false)
11341138
@GenerateUncached
1135-
@ImportStatic(CApiGuards.class)
1136-
public abstract static class AddRefCntNode extends PNodeWithContext {
1139+
public abstract static class DecRefPointerNode extends PNodeWithContext {
11371140

1138-
public static Object executeUncached(Object object, long value) {
1139-
return AddRefCntNodeGen.getUncached().execute(null, object, value);
1141+
public static void executeUncached(Object pointer) {
1142+
CExtNodesFactory.DecRefPointerNodeGen.getUncached().execute(null, pointer);
11401143
}
11411144

1142-
public abstract Object execute(Node inliningTarget, Object object, long value);
1143-
1144-
@Specialization(guards = "value == 1")
1145-
static PythonAbstractObjectNativeWrapper doNativeWrapperIncByOne(Node inliningTarget, PythonAbstractObjectNativeWrapper nativeWrapper, @SuppressWarnings("unused") long value,
1146-
@Shared @Cached InlinedConditionProfile isNativeProfile) {
1147-
if (nativeWrapper.isNative(inliningTarget, isNativeProfile)) {
1148-
nativeWrapper.incRef();
1149-
}
1150-
return nativeWrapper;
1151-
}
1145+
public abstract void execute(Node inliningTarget, Object pointer);
11521146

1153-
@Specialization(replaces = "doNativeWrapperIncByOne")
1154-
static PythonAbstractObjectNativeWrapper doNativeWrapper(Node inliningTarget, PythonAbstractObjectNativeWrapper nativeWrapper, long value,
1155-
@Shared @Cached InlinedConditionProfile isNativeProfile,
1156-
@Exclusive @Cached InlinedConditionProfile hasRefProfile) {
1157-
assert value >= 0 : "adding negative reference count; dealloc might not happen";
1158-
if (nativeWrapper.isNative(inliningTarget, isNativeProfile)) {
1159-
nativeWrapper.setRefCount(inliningTarget, nativeWrapper.getRefCount() + value, hasRefProfile);
1147+
@Specialization
1148+
static void doDecref(Node inliningTarget, Object pointerObj,
1149+
@CachedLibrary(limit = "2") InteropLibrary lib,
1150+
@Cached(inline = false) CApiTransitions.ToPythonWrapperNode toPythonWrapperNode,
1151+
@Cached InlinedBranchProfile isWrapperProfile,
1152+
@Cached InlinedBranchProfile isNativeObject,
1153+
@Cached(inline = false) CStructAccess.ReadI64Node readRefcount,
1154+
@Cached(inline = false) CStructAccess.WriteLongNode writeRefcount,
1155+
@Cached(inline = false) PCallCapiFunction callDealloc) {
1156+
long pointer;
1157+
if (pointerObj instanceof Long longPointer) {
1158+
pointer = longPointer;
1159+
} else {
1160+
if (lib.isPointer(pointerObj)) {
1161+
try {
1162+
pointer = lib.asPointer(pointerObj);
1163+
} catch (UnsupportedMessageException e) {
1164+
throw CompilerDirectives.shouldNotReachHere();
1165+
}
1166+
} else {
1167+
// No refcounting in managed mode
1168+
return;
1169+
}
11601170
}
1161-
return nativeWrapper;
1162-
}
1163-
1164-
@Specialization(guards = "!isNativeWrapper(object)", limit = "2")
1165-
static Object doNativeObject(Object object, long value,
1166-
@CachedLibrary("object") InteropLibrary lib) {
1167-
CApiContext cApiContext = PythonContext.get(lib).getCApiContext();
1168-
if (cApiContext != null && !lib.isNull(object)) {
1169-
assert value >= 0 : "adding negative reference count; dealloc might not happen";
1170-
try {
1171-
long pointer = lib.asPointer(object);
1172-
cApiContext.checkAccess(object, lib);
1173-
long refCount = CApiTransitions.readNativeRefCount(pointer);
1174-
CApiTransitions.writeNativeRefCount(pointer, refCount + value);
1175-
} catch (UnsupportedMessageException e) {
1176-
throw shouldNotReachHere(e);
1171+
PythonNativeWrapper wrapper = toPythonWrapperNode.executeWrapper(pointer, false);
1172+
if (wrapper instanceof PythonAbstractObjectNativeWrapper objectWrapper) {
1173+
isWrapperProfile.enter(inliningTarget);
1174+
objectWrapper.decRef();
1175+
} else if (wrapper == null) {
1176+
isNativeObject.enter(inliningTarget);
1177+
assert NativeToPythonNode.executeUncached(new NativePointer(pointer)) instanceof PythonAbstractNativeObject;
1178+
long refcount = readRefcount.read(pointer, PyObject__ob_refcnt);
1179+
if (refcount != IMMORTAL_REFCNT) {
1180+
refcount--;
1181+
writeRefcount.write(pointer, PyObject__ob_refcnt, refcount);
1182+
if (refcount == 0) {
1183+
callDealloc.call(FUN_PY_DEALLOC, pointer);
1184+
}
11771185
}
1186+
} else {
1187+
throw CompilerDirectives.shouldNotReachHere("Cannot DECREF non-object");
11781188
}
1179-
return object;
11801189
}
11811190
}
11821191

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/ExternalFunctionNodes.java

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@
6363
import com.oracle.graal.python.builtins.PythonBuiltins;
6464
import com.oracle.graal.python.builtins.objects.PNone;
6565
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
66-
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
67-
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PCallCapiFunction;
6866
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.ReleaseNativeWrapperNode;
6967
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.AsCharPointerNodeGen;
7068
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.ReleaseNativeWrapperNodeGen;
@@ -73,13 +71,11 @@
7371
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.ExternalFunctionInvokeNodeGen;
7472
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.MaterializePrimitiveNodeGen;
7573
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.ReleaseNativeSequenceStorageNodeGen;
76-
import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper;
7774
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor;
7875
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
7976
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
8077
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.NativeToPythonNode;
8178
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.PythonToNativeNode;
82-
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.ToPythonWrapperNode;
8379
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitionsFactory.PythonToNativeNodeGen;
8480
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.CheckFunctionResultNode;
8581
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.ClearCurrentExceptionNode;
@@ -2233,46 +2229,28 @@ abstract static class ReleaseNativeSequenceStorageNode extends Node {
22332229
@Specialization(guards = {"storage.length() == cachedLen", "cachedLen <= 8"}, limit = "1")
22342230
@ExplodeLoop(kind = LoopExplosionKind.FULL_UNROLL)
22352231
static void doObjectCachedLen(NativeObjectSequenceStorage storage,
2232+
@Bind("this") Node inliningTarget,
22362233
@Cached("storage.length()") int cachedLen,
2237-
@Cached(value = "createConditionProfiles(cachedLen)", dimensions = 1) ConditionProfile[] objectWrapperProfiles,
2238-
@Cached(value = "createConditionProfiles(cachedLen)", dimensions = 1) ConditionProfile[] nativeObjectProfiles,
22392234
@Shared @Cached CStructAccess.ReadPointerNode readNode,
2240-
@Shared @Cached ToPythonWrapperNode toPythonWrapperNode,
2241-
@Shared @Cached PCallCapiFunction callDecrefNode,
2235+
@Shared @Cached CExtNodes.DecRefPointerNode decRefPointerNode,
22422236
@Shared @Cached CStructAccess.FreeNode freeNode) {
22432237
for (int i = 0; i < cachedLen; i++) {
22442238
Object elementPointer = readNode.readArrayElement(storage.getPtr(), i);
2245-
PythonNativeWrapper pythonNativeWrapper = toPythonWrapperNode.executeWrapper(elementPointer, false);
2246-
if (objectWrapperProfiles[i].profile(pythonNativeWrapper instanceof PythonAbstractObjectNativeWrapper)) {
2247-
((PythonAbstractObjectNativeWrapper) pythonNativeWrapper).decRef();
2248-
} else if (nativeObjectProfiles[i].profile(pythonNativeWrapper == null)) {
2249-
assert NativeToPythonNode.executeUncached(elementPointer) instanceof PythonAbstractNativeObject;
2250-
callDecrefNode.call(NativeCAPISymbol.FUN_DECREF, elementPointer);
2251-
} else {
2252-
throw CompilerDirectives.shouldNotReachHere("NativeObjectSequenceStorage contains non-object element");
2253-
}
2239+
decRefPointerNode.execute(inliningTarget, elementPointer);
22542240
}
22552241
// in this case, the runtime still exclusively owns the memory
22562242
freeNode.free(storage.getPtr());
22572243
}
22582244

22592245
@Specialization(replaces = "doObjectCachedLen")
22602246
static void doObjectGeneric(NativeObjectSequenceStorage storage,
2247+
@Bind("this") Node inliningTarget,
22612248
@Shared @Cached CStructAccess.ReadPointerNode readNode,
2262-
@Shared @Cached ToPythonWrapperNode toPythonWrapperNode,
2263-
@Shared @Cached PCallCapiFunction callDecrefNode,
2249+
@Shared @Cached CExtNodes.DecRefPointerNode decRefPointerNode,
22642250
@Shared @Cached CStructAccess.FreeNode freeNode) {
22652251
for (int i = 0; i < storage.length(); i++) {
22662252
Object elementPointer = readNode.readArrayElement(storage.getPtr(), i);
2267-
PythonNativeWrapper pythonNativeWrapper = toPythonWrapperNode.executeWrapper(elementPointer, false);
2268-
if (pythonNativeWrapper instanceof PythonAbstractObjectNativeWrapper objectNativeWrapper) {
2269-
objectNativeWrapper.decRef();
2270-
} else if (pythonNativeWrapper == null) {
2271-
assert NativeToPythonNode.executeUncached(elementPointer) instanceof PythonAbstractNativeObject;
2272-
callDecrefNode.call(NativeCAPISymbol.FUN_DECREF, elementPointer);
2273-
} else {
2274-
throw CompilerDirectives.shouldNotReachHere("NativeObjectSequenceStorage contains non-object element");
2275-
}
2253+
decRefPointerNode.execute(inliningTarget, elementPointer);
22762254
}
22772255
// in this case, the runtime still exclusively owns the memory
22782256
freeNode.free(storage.getPtr());

0 commit comments

Comments
 (0)