Skip to content

Commit 87a62a8

Browse files
fangererfangerer
committed
[GR-22767] Cleanup native memory at finalization.
PullRequest: graalpython/3253
2 parents e9278f8 + d53e439 commit 87a62a8

File tree

15 files changed

+419
-171
lines changed

15 files changed

+419
-171
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/PythonLanguage.java

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,6 @@
128128
import com.oracle.truffle.api.library.ExportMessage;
129129
import com.oracle.truffle.api.nodes.DirectCallNode;
130130
import com.oracle.truffle.api.nodes.ExecutableNode;
131-
import com.oracle.truffle.api.nodes.ExplodeLoop;
132-
import com.oracle.truffle.api.nodes.ExplodeLoop.LoopExplosionKind;
133131
import com.oracle.truffle.api.nodes.Node;
134132
import com.oracle.truffle.api.nodes.RootNode;
135133
import com.oracle.truffle.api.object.Shape;
@@ -345,7 +343,8 @@ public boolean isSingleContext() {
345343
private final Shape emptyShape = Shape.newBuilder().allowImplicitCastIntToDouble(false).allowImplicitCastIntToLong(true).shapeFlags(0).propertyAssumptions(true).build();
346344
@CompilationFinal(dimensions = 1) private final Shape[] builtinTypeInstanceShapes = new Shape[PythonBuiltinClassType.VALUES.length];
347345

348-
@CompilationFinal(dimensions = 1) private static final Object[] CONTEXT_INSENSITIVE_SINGLETONS = new Object[]{PNone.NONE, PNone.NO_VALUE, PEllipsis.INSTANCE, PNotImplemented.NOT_IMPLEMENTED};
346+
@CompilationFinal(dimensions = 1) public static final PythonAbstractObject[] CONTEXT_INSENSITIVE_SINGLETONS = new PythonAbstractObject[]{PNone.NONE, PNone.NO_VALUE, PEllipsis.INSTANCE,
347+
PNotImplemented.NOT_IMPLEMENTED};
349348

350349
/**
351350
* Named semaphores are shared between all processes in a system, and they persist until the
@@ -381,20 +380,6 @@ public static PythonLanguage get(Node node) {
381380
return REFERENCE.get(node);
382381
}
383382

384-
public static int getNumberOfSpecialSingletons() {
385-
return CONTEXT_INSENSITIVE_SINGLETONS.length;
386-
}
387-
388-
@ExplodeLoop(kind = LoopExplosionKind.FULL_UNROLL_UNTIL_RETURN)
389-
public static int getSingletonNativeWrapperIdx(Object obj) {
390-
for (int i = 0; i < CONTEXT_INSENSITIVE_SINGLETONS.length; i++) {
391-
if (CONTEXT_INSENSITIVE_SINGLETONS[i] == obj) {
392-
return i;
393-
}
394-
}
395-
return -1;
396-
}
397-
398383
/**
399384
* <b>DO NOT DIRECTLY USE THIS METHOD !!!</b> Instead, use
400385
* {@link PythonContext#getThreadState(PythonLanguage)}}.
@@ -1019,7 +1004,7 @@ public RootCallTarget getDescriptorCallTarget(BuiltinMethodDescriptor descriptor
10191004
@Override
10201005
protected void exitContext(PythonContext context, ExitMode exitMode, int exitCode) {
10211006
if (context.getCApiContext() != null) {
1022-
context.getCApiContext().finalizeCapi();
1007+
context.getCApiContext().exitCApiContext();
10231008
}
10241009
if (!PythonOptions.WITHOUT_PLATFORM_ACCESS && !ImageInfo.inImageBuildtimeCode()) {
10251010
// Reset signal handlers back to what they were

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

Lines changed: 171 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
*/
4141
package com.oracle.graal.python.builtins.objects.cext.capi;
4242

43+
import static com.oracle.graal.python.PythonLanguage.CONTEXT_INSENSITIVE_SINGLETONS;
44+
import static com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper.IMMORTAL_REFCNT;
4345
import static com.oracle.graal.python.nodes.SpecialAttributeNames.T___FILE__;
4446
import static com.oracle.graal.python.nodes.SpecialAttributeNames.T___LIBRARY__;
4547
import static com.oracle.graal.python.nodes.StringLiterals.J_LLVM_LANGUAGE;
@@ -72,11 +74,14 @@
7274
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiBuiltinExecutable;
7375
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath;
7476
import com.oracle.graal.python.builtins.objects.PNone;
77+
import com.oracle.graal.python.builtins.objects.PythonAbstractObject;
7578
import com.oracle.graal.python.builtins.objects.capsule.PyCapsule;
7679
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.CreateModuleNodeGen;
80+
import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper;
7781
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
82+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.HandleContext;
7883
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.NativeToPythonNode;
79-
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.NativeToPythonStealingNode;
84+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.ToPythonWrapperNode;
8085
import com.oracle.graal.python.builtins.objects.cext.common.CExtCommonNodes.CheckFunctionResultNode;
8186
import com.oracle.graal.python.builtins.objects.cext.common.CExtContext;
8287
import com.oracle.graal.python.builtins.objects.cext.common.LoadCExtException.ApiInitException;
@@ -118,6 +123,7 @@
118123
import com.oracle.truffle.api.TruffleFile;
119124
import com.oracle.truffle.api.TruffleLanguage.Env;
120125
import com.oracle.truffle.api.TruffleLogger;
126+
import com.oracle.truffle.api.TruffleSafepoint;
121127
import com.oracle.truffle.api.frame.VirtualFrame;
122128
import com.oracle.truffle.api.interop.ArityException;
123129
import com.oracle.truffle.api.interop.InteropLibrary;
@@ -127,6 +133,8 @@
127133
import com.oracle.truffle.api.interop.UnsupportedTypeException;
128134
import com.oracle.truffle.api.library.ExportLibrary;
129135
import com.oracle.truffle.api.library.ExportMessage;
136+
import com.oracle.truffle.api.nodes.ExplodeLoop;
137+
import com.oracle.truffle.api.nodes.ExplodeLoop.LoopExplosionKind;
130138
import com.oracle.truffle.api.nodes.Node;
131139
import com.oracle.truffle.api.source.Source;
132140
import com.oracle.truffle.api.source.Source.SourceBuilder;
@@ -139,6 +147,12 @@ public final class CApiContext extends CExtContext {
139147

140148
public static final String LOGGER_CAPI_NAME = "capi";
141149

150+
/** Same as _PY_NSMALLNEGINTS */
151+
public static final int PY_NSMALLNEGINTS = 5;
152+
153+
/** Same as _PY_NSMALLPOSINTS */
154+
public static final int PY_NSMALLPOSINTS = 257;
155+
142156
/**
143157
* NFI source for Python module init functions (i.e. {@code "PyInit_modname"}).
144158
*/
@@ -162,6 +176,9 @@ public final class CApiContext extends CExtContext {
162176
/** Container of pointers that have seen to be free'd. */
163177
private Map<Object, AllocInfo> freedNativeMemory;
164178

179+
/** Native wrappers for context-insensitive singletons like {@link PNone#NONE}. */
180+
@CompilationFinal(dimensions = 1) private final PythonAbstractObjectNativeWrapper[] singletonNativePtrs;
181+
165182
/**
166183
* This cache is used to cache native wrappers for frequently used primitives. This is strictly
167184
* defined to be the range {@code [-5, 256]}. CPython does exactly the same (see
@@ -283,10 +300,22 @@ public CApiContext(PythonContext context, Object llvmLibrary, boolean useNativeB
283300
CApiContext.nativeSymbolCacheSingleContextUsed = true;
284301
}
285302

303+
// initialize singleton native wrappers
304+
singletonNativePtrs = new PythonAbstractObjectNativeWrapper[CONTEXT_INSENSITIVE_SINGLETONS.length];
305+
// Other threads must see the nativeWrapper fully initialized once it becomes non-null
306+
for (int i = 0; i < singletonNativePtrs.length; i++) {
307+
assert CApiGuards.isSpecialSingleton(CONTEXT_INSENSITIVE_SINGLETONS[i]);
308+
/*
309+
* Note: this does intentionally not use 'PythonObjectNativeWrapper.wrap' because the
310+
* wrapper must not be reachable from the Python object since the singletons are shared.
311+
*/
312+
singletonNativePtrs[i] = new PythonObjectNativeWrapper(CONTEXT_INSENSITIVE_SINGLETONS[i]);
313+
}
314+
286315
// initialize primitive native wrapper cache
287-
primitiveNativeWrapperCache = new PrimitiveNativeWrapper[262];
316+
primitiveNativeWrapperCache = new PrimitiveNativeWrapper[PY_NSMALLNEGINTS + PY_NSMALLPOSINTS];
288317
for (int i = 0; i < primitiveNativeWrapperCache.length; i++) {
289-
int value = i - 5;
318+
int value = i - PY_NSMALLNEGINTS;
290319
assert CApiGuards.isSmallInteger(value);
291320
primitiveNativeWrapperCache[i] = PrimitiveNativeWrapper.createInt(value);
292321
}
@@ -375,6 +404,46 @@ public void tssDelete(long key) {
375404
tssStorage.remove(key);
376405
}
377406

407+
@ExplodeLoop(kind = LoopExplosionKind.FULL_UNROLL_UNTIL_RETURN)
408+
static int getSingletonNativeWrapperIdx(Object obj) {
409+
for (int i = 0; i < CONTEXT_INSENSITIVE_SINGLETONS.length; i++) {
410+
if (CONTEXT_INSENSITIVE_SINGLETONS[i] == obj) {
411+
return i;
412+
}
413+
}
414+
return -1;
415+
}
416+
417+
public PythonAbstractObjectNativeWrapper getSingletonNativeWrapper(PythonAbstractObject obj) {
418+
int singletonNativePtrIdx = CApiContext.getSingletonNativeWrapperIdx(obj);
419+
if (singletonNativePtrIdx != -1) {
420+
return singletonNativePtrs[singletonNativePtrIdx];
421+
}
422+
return null;
423+
}
424+
425+
/**
426+
* Deallocates all singleton wrappers (in {@link #singletonNativePtrs}) which are immortal and
427+
* must therefore be explicitly free'd. This method modifies the
428+
* {@link HandleContext#nativeStubLookup stub lookup table} but runs not guest code.
429+
*/
430+
private void freeSingletonNativeWrappers(HandleContext handleContext) {
431+
CompilerAsserts.neverPartOfCompilation();
432+
// TODO(fa): this should not require the GIL (GR-51314)
433+
assert getContext().ownsGil();
434+
for (int i = 0; i < singletonNativePtrs.length; i++) {
435+
PythonAbstractObjectNativeWrapper singletonNativeWrapper = singletonNativePtrs[i];
436+
singletonNativePtrs[i] = null;
437+
assert singletonNativeWrapper != null;
438+
assert getSingletonNativeWrapperIdx(singletonNativeWrapper.getDelegate()) != -1;
439+
assert !singletonNativeWrapper.isNative() || singletonNativeWrapper.getRefCount() == IMMORTAL_REFCNT;
440+
if (singletonNativeWrapper.ref != null) {
441+
CApiTransitions.nativeStubLookupRemove(handleContext, singletonNativeWrapper.ref);
442+
}
443+
PyTruffleObjectFree.releaseNativeWrapperUncached(singletonNativeWrapper);
444+
}
445+
}
446+
378447
public PrimitiveNativeWrapper getCachedPrimitiveNativeWrapper(int i) {
379448
assert CApiGuards.isSmallInteger(i);
380449
PrimitiveNativeWrapper primitiveNativeWrapper = primitiveNativeWrapperCache[i + 5];
@@ -396,33 +465,61 @@ Object getOrCreateSmallInts() {
396465
// TODO(fa): this should not require the GIL (GR-51314)
397466
assert getContext().ownsGil();
398467
if (nativeSmallIntsArray == null) {
399-
int nSmallNegativeInts = CConstants._PY_NSMALLNEGINTS.intValue();
400-
int nSmallPositiveInts = CConstants._PY_NSMALLPOSINTS.intValue();
401-
Object smallInts = CStructAccess.AllocateNode.callocUncached(nSmallNegativeInts + nSmallPositiveInts, CStructAccess.POINTER_SIZE);
402-
for (int i = 0; i < nSmallNegativeInts + nSmallPositiveInts; i++) {
403-
CStructAccessFactory.WriteObjectNewRefNodeGen.getUncached().writeArrayElement(smallInts, i, i - nSmallNegativeInts);
468+
assert CConstants._PY_NSMALLNEGINTS.intValue() == PY_NSMALLNEGINTS;
469+
assert CConstants._PY_NSMALLPOSINTS.intValue() == PY_NSMALLPOSINTS;
470+
Object smallInts = CStructAccess.AllocateNode.callocUncached(PY_NSMALLNEGINTS + PY_NSMALLPOSINTS, CStructAccess.POINTER_SIZE);
471+
for (int i = 0; i < PY_NSMALLNEGINTS + PY_NSMALLPOSINTS; i++) {
472+
CStructAccessFactory.WriteObjectNewRefNodeGen.getUncached().writeArrayElement(smallInts, i, i - PY_NSMALLNEGINTS);
404473
}
405474
nativeSmallIntsArray = smallInts;
406475
}
407476
return nativeSmallIntsArray;
408477
}
409478

410-
private void freeSmallInts() {
479+
/**
480+
* Deallocates the native small int array (pointer {@link #nativeSmallIntsArray}) and all
481+
* wrappers of the small ints (in {@link #primitiveNativeWrapperCache}) which are immortal and
482+
* must therefore be explicitly free'd. This method modifies the
483+
* {@link HandleContext#nativeStubLookup stub lookup table} but runs not guest code.
484+
*/
485+
private void freeSmallInts(HandleContext handleContext) {
411486
CompilerAsserts.neverPartOfCompilation();
412487
// TODO(fa): this should not require the GIL (GR-51314)
413488
assert getContext().ownsGil();
414489
if (nativeSmallIntsArray != null) {
415-
int nSmallNegativeInts = CConstants._PY_NSMALLNEGINTS.intValue();
416-
int nSmallPositiveInts = CConstants._PY_NSMALLPOSINTS.intValue();
417-
for (int i = 0; i < nSmallNegativeInts + nSmallPositiveInts; i++) {
418-
Object elementPtr = ReadPointerNode.getUncached().readArrayElement(nativeSmallIntsArray, i);
419-
// again, take ownership
420-
Object element = NativeToPythonStealingNode.executeUncached(elementPtr);
421-
assert element instanceof Number number && number.intValue() == i - nSmallNegativeInts;
422-
}
490+
assert verifyNativeSmallInts();
491+
// free the native array used to store the stub pointers of the small int wrappers
423492
FreeNode.executeUncached(nativeSmallIntsArray);
424493
nativeSmallIntsArray = null;
425494
}
495+
for (PrimitiveNativeWrapper wrapper : primitiveNativeWrapperCache) {
496+
assert wrapper.isIntLike() && CApiGuards.isSmallLong(wrapper.getLong());
497+
assert !wrapper.isNative() || wrapper.getRefCount() == IMMORTAL_REFCNT;
498+
if (wrapper.ref != null) {
499+
CApiTransitions.nativeStubLookupRemove(handleContext, wrapper.ref);
500+
}
501+
PyTruffleObjectFree.releaseNativeWrapperUncached(wrapper);
502+
}
503+
}
504+
505+
/**
506+
* Verifies integrity of the pointers stored in the native small int array. Each pointer must
507+
* denote the according small int wrapper. The objects are expected to be immortal.
508+
*/
509+
private boolean verifyNativeSmallInts() {
510+
// TODO(fa): this should not require the GIL (GR-51314)
511+
assert getContext().ownsGil();
512+
for (int i = 0; i < PY_NSMALLNEGINTS + PY_NSMALLPOSINTS; i++) {
513+
Object elementPtr = ReadPointerNode.getUncached().readArrayElement(nativeSmallIntsArray, i);
514+
PythonNativeWrapper wrapper = ToPythonWrapperNode.executeUncached(elementPtr, false);
515+
if (wrapper != primitiveNativeWrapperCache[i]) {
516+
return false;
517+
}
518+
if (primitiveNativeWrapperCache[i].isNative() && primitiveNativeWrapperCache[i].getRefCount() != IMMORTAL_REFCNT) {
519+
return false;
520+
}
521+
}
522+
return true;
426523
}
427524

428525
public Object getModuleByIndex(int i) {
@@ -798,15 +895,65 @@ private void addNativeFinalizer(Env env, Object resetFunctionPointerArray) {
798895
}
799896
}
800897

801-
@TruffleBoundary
898+
/**
899+
* This method is called to exit the context assuming a
900+
* {@link com.oracle.truffle.api.TruffleLanguage.ExitMode#NATURAL natural exit}. This means, it
901+
* is allowed to run guest code. Hence, we deallocate any reachable native object here since
902+
* they may have custom {@code tp_dealloc} functions.
903+
*/
802904
@SuppressWarnings("try")
803-
public void finalizeCapi() {
804-
// TODO(fa): remove GIL acquisition (GR-51314)
805-
try (GilNode.UncachedAcquire gil = GilNode.uncachedAcquire()) {
806-
freeSmallInts();
905+
public void exitCApiContext() {
906+
CompilerAsserts.neverPartOfCompilation();
907+
/*
908+
* Polling the native reference queue is the only task we can do here because deallocating
909+
* objects may run arbitrary guest code that can again call into the interpreter.
910+
*/
911+
CApiTransitions.pollReferenceQueue();
912+
/*
913+
* Deallocating native storages and objects may run arbitrary guest code. So, we need to
914+
* ensure that the GIL is held.
915+
*/
916+
try (GilNode.UncachedAcquire ignored = GilNode.uncachedAcquire()) {
917+
CApiTransitions.deallocateNativeWeakRefs(getContext());
807918
}
808-
if (pyDateTimeCAPICapsule != null) {
809-
PyDateTimeCAPIWrapper.destroyWrapper(pyDateTimeCAPICapsule);
919+
}
920+
921+
@SuppressWarnings("try")
922+
public void finalizeCApi() {
923+
CompilerAsserts.neverPartOfCompilation();
924+
HandleContext handleContext = getContext().nativeContext;
925+
/*
926+
* Disable reference queue polling because during finalization, we will free any known
927+
* allocated resources (e.g. native object stubs). Calling
928+
* 'CApiTransitions.pollReferenceQueue' could then lead to a double-free.
929+
*/
930+
CApiTransitions.disableReferenceQueuePolling(handleContext);
931+
932+
TruffleSafepoint sp = TruffleSafepoint.getCurrent();
933+
boolean prev = sp.setAllowActions(false);
934+
try {
935+
// TODO(fa): remove GIL acquisition (GR-51314)
936+
try (GilNode.UncachedAcquire ignored = GilNode.uncachedAcquire()) {
937+
freeSmallInts(handleContext);
938+
freeSingletonNativeWrappers(handleContext);
939+
/*
940+
* Clear all remaining native object stubs. This must be done after the small int
941+
* and the singleton wrappers were cleared because they might also end up in the
942+
* lookup table and may otherwise be double-freed.
943+
*/
944+
CApiTransitions.freeNativeObjectStubs(handleContext);
945+
CApiTransitions.freeClassReplacements(handleContext);
946+
CApiTransitions.freeNativeStorages(handleContext);
947+
}
948+
if (pyDateTimeCAPICapsule != null) {
949+
PyDateTimeCAPIWrapper.destroyWrapper(pyDateTimeCAPICapsule);
950+
}
951+
// free all allocated PyMethodDef structures
952+
for (Object pyMethodDefPointer : methodDefinitions.values()) {
953+
PyMethodDefHelper.free(pyMethodDefPointer);
954+
}
955+
} finally {
956+
sp.setAllowActions(prev);
810957
}
811958
if (nativeFinalizerShutdownHook != null) {
812959
try {
@@ -817,10 +964,6 @@ public void finalizeCapi() {
817964
}
818965
}
819966
pyCFunctionWrappers.clear();
820-
// free all allocated PyMethodDef structures
821-
for (Object pyMethodDefPointer : methodDefinitions.values()) {
822-
PyMethodDefHelper.free(pyMethodDefPointer);
823-
}
824967
/*
825968
* If the static symbol cache is not null, then it is guaranteed that this context instance
826969
* was the exclusive user of it. We can now reset the state such that other contexts created

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
*/
4141
package com.oracle.graal.python.builtins.objects.cext.capi;
4242

43-
import com.oracle.graal.python.PythonLanguage;
4443
import com.oracle.graal.python.builtins.objects.cext.common.NativePointer;
4544

4645
public abstract class CApiGuards {
@@ -58,20 +57,20 @@ public static boolean isNativeNull(Object object) {
5857
}
5958

6059
public static boolean isSpecialSingleton(Object delegate) {
61-
return PythonLanguage.getSingletonNativeWrapperIdx(delegate) != -1;
60+
return CApiContext.getSingletonNativeWrapperIdx(delegate) != -1;
6261
}
6362

6463
/**
6564
* This guard defines the range of integer values for which PInt objects (and in our particular
6665
* case, native wrappers) are cached.
6766
*/
6867
public static boolean isSmallInteger(int i) {
69-
return -5 <= i && i < 257;
68+
return -CApiContext.PY_NSMALLNEGINTS <= i && i < CApiContext.PY_NSMALLPOSINTS;
7069
}
7170

7271
/** see {@link #isSmallInteger(int)} */
7372
public static boolean isSmallLong(long i) {
74-
return -5 <= i && i < 257;
73+
return -CApiContext.PY_NSMALLNEGINTS <= i && i < CApiContext.PY_NSMALLPOSINTS;
7574
}
7675

7776
public static boolean isSmallIntegerWrapper(PrimitiveNativeWrapper nativeWrapper) {

0 commit comments

Comments
 (0)