Skip to content

Commit e26fa1e

Browse files
msimacekmsimacek
committed
[GR-53370] Stash exception state when calling native destructors
PullRequest: graalpython/3296
2 parents 4880447 + c74bf64 commit e26fa1e

File tree

14 files changed

+155
-95
lines changed

14 files changed

+155
-95
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ int Py_IsInitialized(void) {
4444
return !graalpy_finalizing;
4545
}
4646

47+
int
48+
_Py_IsFinalizing(void)
49+
{
50+
return graalpy_finalizing;
51+
}
52+
4753
void _Py_NO_RETURN _Py_FatalErrorFunc(const char *func, const char *msg) {
4854
GraalPyTruffle_FatalErrorFunc(func, msg, -1);
4955
/* If the above upcall returns, then we just fall through to the 'abort' call. */

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1715,7 +1715,7 @@ protected abstract static class PyDECREFNode extends PythonUnaryBuiltinNode {
17151715
static Object doGeneric(Object arg) {
17161716
CApiContext.ensureCapiWasLoaded();
17171717
Object nativePointer = CApiTransitions.PythonToNativeNode.executeUncached(arg);
1718-
CExtNodes.DecRefPointerNode.executeUncached(nativePointer);
1718+
CExtNodes.XDecRefPointerNode.executeUncached(nativePointer);
17191719
return arg;
17201720
}
17211721
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,7 @@ public final class CApiFunction {
618618
@CApiBuiltin(name = "_Py_HashPointer", ret = Py_hash_t, args = {CONST_VOID_PTR}, call = CImpl)
619619
@CApiBuiltin(name = "_Py_HashPointerRaw", ret = Py_hash_t, args = {CONST_VOID_PTR}, call = CImpl)
620620
@CApiBuiltin(name = "_Py_IncRef", ret = Void, args = {PyObject}, call = CImpl)
621+
@CApiBuiltin(name = "_Py_IsFinalizing", ret = Int, args = {}, call = CImpl)
621622
@CApiBuiltin(name = "_Py_NewReference", ret = Void, args = {PyObject}, call = CImpl)
622623
@CApiBuiltin(name = "_Py_VaBuildStack", ret = PyObjectPtr, args = {PyObjectPtr, Py_ssize_t, ConstCharPtrAsTruffleString, VA_LIST, PY_SSIZE_T_PTR}, call = CImpl)
623624
@CApiBuiltin(name = "_Py_VaBuildStack_SizeT", ret = PyObjectPtr, args = {PyObjectPtr, Py_ssize_t, ConstCharPtrAsTruffleString, VA_LIST, PY_SSIZE_T_PTR}, call = CImpl)
@@ -1215,7 +1216,6 @@ public final class CApiFunction {
12151216
@CApiBuiltin(name = "_Py_GetConfig", ret = CONST_PYCONFIG_PTR, args = {}, call = NotImplemented)
12161217
@CApiBuiltin(name = "_Py_InitializeMain", ret = PYSTATUS, args = {}, call = NotImplemented)
12171218
@CApiBuiltin(name = "_Py_IsCoreInitialized", ret = Int, args = {}, call = NotImplemented)
1218-
@CApiBuiltin(name = "_Py_IsFinalizing", ret = Int, args = {}, call = NotImplemented)
12191219
@CApiBuiltin(name = "_Py_LegacyLocaleDetected", ret = Int, args = {Int}, call = NotImplemented)
12201220
@CApiBuiltin(name = "_Py_NewInterpreter", ret = PyThreadState, args = {Int}, call = NotImplemented)
12211221
@CApiBuiltin(name = "_Py_RestoreSignals", ret = Void, args = {}, call = NotImplemented)

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,10 +1136,10 @@ static PRaiseNativeNode doIt(@Cached(inline = false) PRaiseNativeNode node) {
11361136
@GenerateInline
11371137
@GenerateCached(false)
11381138
@GenerateUncached
1139-
public abstract static class DecRefPointerNode extends PNodeWithContext {
1139+
public abstract static class XDecRefPointerNode extends PNodeWithContext {
11401140

11411141
public static void executeUncached(Object pointer) {
1142-
CExtNodesFactory.DecRefPointerNodeGen.getUncached().execute(null, pointer);
1142+
CExtNodesFactory.XDecRefPointerNodeGen.getUncached().execute(null, pointer);
11431143
}
11441144

11451145
public abstract void execute(Node inliningTarget, Object pointer);
@@ -1168,6 +1168,9 @@ static void doDecref(Node inliningTarget, Object pointerObj,
11681168
return;
11691169
}
11701170
}
1171+
if (pointer == 0) {
1172+
return;
1173+
}
11711174
PythonNativeWrapper wrapper = toPythonWrapperNode.executeWrapper(pointer, false);
11721175
if (wrapper instanceof PythonAbstractObjectNativeWrapper objectWrapper) {
11731176
isWrapperProfile.enter(inliningTarget);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2232,7 +2232,7 @@ static void doObjectCachedLen(NativeObjectSequenceStorage storage,
22322232
@Bind("this") Node inliningTarget,
22332233
@Cached("storage.length()") int cachedLen,
22342234
@Shared @Cached CStructAccess.ReadPointerNode readNode,
2235-
@Shared @Cached CExtNodes.DecRefPointerNode decRefPointerNode,
2235+
@Shared @Cached CExtNodes.XDecRefPointerNode decRefPointerNode,
22362236
@Shared @Cached CStructAccess.FreeNode freeNode) {
22372237
for (int i = 0; i < cachedLen; i++) {
22382238
Object elementPointer = readNode.readArrayElement(storage.getPtr(), i);
@@ -2246,7 +2246,7 @@ static void doObjectCachedLen(NativeObjectSequenceStorage storage,
22462246
static void doObjectGeneric(NativeObjectSequenceStorage storage,
22472247
@Bind("this") Node inliningTarget,
22482248
@Shared @Cached CStructAccess.ReadPointerNode readNode,
2249-
@Shared @Cached CExtNodes.DecRefPointerNode decRefPointerNode,
2249+
@Shared @Cached CExtNodes.XDecRefPointerNode decRefPointerNode,
22502250
@Shared @Cached CStructAccess.FreeNode freeNode) {
22512251
for (int i = 0; i < storage.length(); i++) {
22522252
Object elementPointer = readNode.readArrayElement(storage.getPtr(), i);

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import com.oracle.graal.python.builtins.objects.cext.structs.CStructAccessFactory;
5050
import com.oracle.graal.python.builtins.objects.cext.structs.CStructs;
5151
import com.oracle.graal.python.builtins.objects.dict.PDict;
52+
import com.oracle.graal.python.nodes.object.GetClassNode;
5253
import com.oracle.graal.python.runtime.PythonContext;
5354
import com.oracle.graal.python.runtime.PythonContext.PythonThreadState;
5455
import com.oracle.truffle.api.CompilerDirectives;
@@ -113,6 +114,11 @@ private static Object allocateCLayout(PythonThreadState threadState) {
113114
writePtrNode.write(ptr, CFields.PyThreadState__dict, toNative.execute(threadStateDict));
114115
CApiContext cApiContext = PythonContext.get(null).getCApiContext();
115116
writePtrNode.write(ptr, CFields.PyThreadState__small_ints, cApiContext.getOrCreateSmallInts());
117+
if (threadState.getCurrentException() != null) {
118+
// See TransformExceptionToNativeNode
119+
Object exceptionType = GetClassNode.executeUncached(threadState.getCurrentException().getUnreifiedException());
120+
CStructAccess.WritePointerNode.getUncached().write(ptr, CFields.PyThreadState__curexc_type, PythonToNativeNode.getUncached().execute(exceptionType));
121+
}
116122
return ptr;
117123
}
118124
}

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

Lines changed: 88 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.FromCharPointerNode;
6464
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PCallCapiFunction;
6565
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
66+
import com.oracle.graal.python.builtins.objects.cext.capi.PThreadState;
6667
import com.oracle.graal.python.builtins.objects.cext.capi.PrimitiveNativeWrapper;
6768
import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper;
6869
import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper;
@@ -85,13 +86,15 @@
8586
import com.oracle.graal.python.builtins.objects.cext.structs.CStructs;
8687
import com.oracle.graal.python.builtins.objects.floats.PFloat;
8788
import com.oracle.graal.python.builtins.objects.getsetdescriptor.DescriptorDeleteMarker;
89+
import com.oracle.graal.python.builtins.objects.traceback.LazyTraceback;
8890
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
8991
import com.oracle.graal.python.nodes.PGuards;
9092
import com.oracle.graal.python.nodes.PNodeWithContext;
9193
import com.oracle.graal.python.nodes.PRaiseNode;
9294
import com.oracle.graal.python.nodes.object.GetClassNode;
9395
import com.oracle.graal.python.runtime.GilNode;
9496
import com.oracle.graal.python.runtime.PythonContext;
97+
import com.oracle.graal.python.runtime.exception.PException;
9598
import com.oracle.graal.python.runtime.sequence.storage.NativeSequenceStorage;
9699
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
97100
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage.StorageType;
@@ -355,69 +358,99 @@ public static void registerNativeSequenceStorage(NativeSequenceStorage storage)
355358
@TruffleBoundary
356359
@SuppressWarnings("try")
357360
public static void pollReferenceQueue() {
358-
HandleContext context = getContext();
359-
if (!context.referenceQueuePollActive) {
361+
PythonContext context = PythonContext.get(null);
362+
HandleContext handleContext = context.nativeContext;
363+
if (!handleContext.referenceQueuePollActive) {
360364
try (GilNode.UncachedAcquire ignored = GilNode.uncachedAcquire()) {
361-
ReferenceQueue<Object> queue = context.referenceQueue;
365+
ReferenceQueue<Object> queue = handleContext.referenceQueue;
362366
int count = 0;
363367
long start = 0;
364-
NativeObjectReferenceArrayWrapper referencesToBeFreed = context.referencesToBeFreed;
365-
while (true) {
366-
Object entry = queue.poll();
367-
if (entry == null) {
368-
if (count > 0) {
369-
assert context.referenceQueuePollActive;
370-
releaseNativeObjects(referencesToBeFreed);
371-
context.referenceQueuePollActive = false;
372-
LOGGER.fine("collected " + count + " references from native reference queue in " + ((System.nanoTime() - start) / 1000000) + "ms");
373-
}
374-
return;
375-
}
376-
if (count == 0) {
377-
assert !context.referenceQueuePollActive;
378-
context.referenceQueuePollActive = true;
379-
start = System.nanoTime();
380-
} else {
381-
assert context.referenceQueuePollActive;
368+
NativeObjectReferenceArrayWrapper referencesToBeFreed = handleContext.referencesToBeFreed;
369+
PythonContext.PythonThreadState threadState = context.getThreadState(context.getLanguage());
370+
/*
371+
* There can be an active exception. Since we might be calling arbitary python, we
372+
* need to stash it.
373+
*/
374+
PException savedException = null;
375+
LazyTraceback savedTraceback = null;
376+
Object savedNativeException = null;
377+
if (threadState.getCurrentException() != null) {
378+
savedException = threadState.getCurrentException();
379+
savedTraceback = threadState.getCurrentTraceback();
380+
threadState.clearCurrentException();
381+
Object nativeThreadState = PThreadState.getNativeThreadState(threadState);
382+
if (nativeThreadState != null) {
383+
savedNativeException = CStructAccess.ReadPointerNode.readUncached(nativeThreadState, CFields.PyThreadState__curexc_type);
384+
CStructAccess.WritePointerNode.writeUncached(nativeThreadState, CFields.PyThreadState__curexc_type, 0L);
382385
}
383-
count++;
384-
if (entry instanceof PythonObjectReference reference) {
385-
LOGGER.fine(() -> PythonUtils.formatJString("releasing %s", reference.toString()));
386-
if (HandlePointerConverter.pointsToPyHandleSpace(reference.pointer)) {
387-
assert nativeStubLookupGet(context, reference.pointer, reference.handleTableIndex) != null : Long.toHexString(reference.pointer);
388-
nativeStubLookupRemove(context, reference);
389-
/*
390-
* We may only free native object stubs if their reference count is
391-
* zero. We cannot free other structs (e.g. PyDateTime_CAPI) because we
392-
* don't know if they are still used from native code. Those must be
393-
* free'd at context finalization.
394-
*/
395-
long stubPointer = HandlePointerConverter.pointerToStub(reference.pointer);
396-
if (subNativeRefCount(stubPointer, MANAGED_REFCNT) == 0) {
397-
freeNativeStub(stubPointer);
398-
} else {
399-
/*
400-
* In this case, the object is no longer referenced from managed but
401-
* still from native code (since the reference count is greater 0).
402-
* We therefore need to overwrite the 'CFields.GraalPyObject__id'
403-
* field because there may be referenced from managed in the future
404-
* and then we would incorrectly reuse the ID.
405-
*/
406-
CStructAccess.WriteIntNode.writeUncached(reference.pointer, CFields.GraalPyObject__handle_table_index, 0);
386+
}
387+
try {
388+
while (true) {
389+
Object entry = queue.poll();
390+
if (entry == null) {
391+
if (count > 0) {
392+
assert handleContext.referenceQueuePollActive;
393+
releaseNativeObjects(referencesToBeFreed);
394+
handleContext.referenceQueuePollActive = false;
395+
LOGGER.fine("collected " + count + " references from native reference queue in " + ((System.nanoTime() - start) / 1000000) + "ms");
407396
}
397+
return;
398+
}
399+
if (count == 0) {
400+
assert !handleContext.referenceQueuePollActive;
401+
handleContext.referenceQueuePollActive = true;
402+
start = System.nanoTime();
408403
} else {
409-
assert nativeLookupGet(context, reference.pointer) != null : Long.toHexString(reference.pointer);
410-
nativeLookupRemove(context, reference.pointer);
411-
if (reference.freeAtCollection) {
412-
freeNativeStruct(reference);
404+
assert handleContext.referenceQueuePollActive;
405+
}
406+
count++;
407+
if (entry instanceof PythonObjectReference reference) {
408+
LOGGER.fine(() -> PythonUtils.formatJString("releasing %s", reference.toString()));
409+
if (HandlePointerConverter.pointsToPyHandleSpace(reference.pointer)) {
410+
assert nativeStubLookupGet(handleContext, reference.pointer, reference.handleTableIndex) != null : Long.toHexString(reference.pointer);
411+
nativeStubLookupRemove(handleContext, reference);
412+
/*
413+
* We may only free native object stubs if their reference count is
414+
* zero. We cannot free other structs (e.g. PyDateTime_CAPI) because
415+
* we don't know if they are still used from native code. Those must
416+
* be free'd at context finalization.
417+
*/
418+
long stubPointer = HandlePointerConverter.pointerToStub(reference.pointer);
419+
if (subNativeRefCount(stubPointer, MANAGED_REFCNT) == 0) {
420+
freeNativeStub(stubPointer);
421+
} else {
422+
/*
423+
* In this case, the object is no longer referenced from managed
424+
* but still from native code (since the reference count is
425+
* greater 0). We therefore need to overwrite the
426+
* 'CFields.GraalPyObject__id' field because there may be
427+
* referenced from managed in the future and then we would
428+
* incorrectly reuse the ID.
429+
*/
430+
CStructAccess.WriteIntNode.writeUncached(reference.pointer, CFields.GraalPyObject__handle_table_index, 0);
431+
}
432+
} else {
433+
assert nativeLookupGet(handleContext, reference.pointer) != null : Long.toHexString(reference.pointer);
434+
nativeLookupRemove(handleContext, reference.pointer);
435+
if (reference.freeAtCollection) {
436+
freeNativeStruct(reference);
437+
}
413438
}
439+
} else if (entry instanceof NativeObjectReference reference) {
440+
nativeLookupRemove(handleContext, reference.pointer);
441+
processNativeObjectReference(reference, referencesToBeFreed);
442+
} else if (entry instanceof NativeStorageReference reference) {
443+
handleContext.nativeStorageReferences.remove(reference);
444+
processNativeStorageReference(reference);
445+
}
446+
}
447+
} finally {
448+
if (savedException != null) {
449+
threadState.setCurrentException(savedException, savedTraceback);
450+
Object nativeThreadState = PThreadState.getNativeThreadState(threadState);
451+
if (nativeThreadState != null) {
452+
CStructAccess.WritePointerNode.writeUncached(nativeThreadState, CFields.PyThreadState__curexc_type, savedNativeException);
414453
}
415-
} else if (entry instanceof NativeObjectReference reference) {
416-
nativeLookupRemove(context, reference.pointer);
417-
processNativeObjectReference(reference, referencesToBeFreed);
418-
} else if (entry instanceof NativeStorageReference reference) {
419-
context.nativeStorageReferences.remove(reference);
420-
processNativeStorageReference(reference);
421454
}
422455
}
423456
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/common/CExtCommonNodes.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,12 @@ static void setCurrentException(Frame frame, Node inliningTarget, PException e,
480480
@Cached GetClassNode getClassNode,
481481
@Cached(inline = false) PythonToNativeNode pythonToNativeNode,
482482
@Cached(inline = false) CStructAccess.WritePointerNode writePointerNode) {
483+
/*
484+
* Run the ToNative conversion early so that nothing interrups the code between setting
485+
* the managed and native states
486+
*/
487+
Object exceptionType = getClassNode.execute(inliningTarget, e.getUnreifiedException());
488+
Object exceptionTypeNative = pythonToNativeNode.execute(exceptionType);
483489
// TODO connect f_back
484490
getCurrentFrameRef.execute(frame, inliningTarget).markAsEscaped();
485491
PythonThreadState threadState = getThreadStateNode.execute(inliningTarget);
@@ -497,12 +503,11 @@ static void setCurrentException(Frame frame, Node inliningTarget, PException e,
497503
*/
498504
Object nativeThreadState = PThreadState.getNativeThreadState(threadState);
499505
if (nativeThreadState != null) {
500-
Object exceptionType = getClassNode.execute(inliningTarget, e.getUnreifiedException());
501506
/*
502507
* Write a borrowed ref to the native mirror because we need to keep that in sync
503508
* anyway.
504509
*/
505-
writePointerNode.write(nativeThreadState, CFields.PyThreadState__curexc_type, pythonToNativeNode.execute(exceptionType));
510+
writePointerNode.write(nativeThreadState, CFields.PyThreadState__curexc_type, exceptionTypeNative);
506511
}
507512
}
508513
}

0 commit comments

Comments
 (0)