Skip to content

Commit 88a5489

Browse files
committed
Mark frames as escaped even when the first escape happens after the transition to cached.
Traceback collection counted frames during exception unwinding, but did not mark them as escaped. In uncached execution, we had the PFrame.Reference links unconditionally, so this worked when it happened then, and we marked the root as needing the PFrame.Reference link, so on later cached calls we have them. But if we raise and escape the frame for the very first time when already cached we did not preserve the f_back chain and could not reconstruct it properly later. This change makes the frames that belong to a traceback mark their current PFrame.Reference as escaped while the exception is unwinding and the VirtualFrame is thus still on the stack. The CalleeContext#exit logic then materializes the frame and propagates to caller frames and this preserves the f_back chain. This can be triggered by forcing immediately cached execution, but a regression test covers the case where functions first transition to cached execution without raising, then later raises after some time (presumably after we transitioned to cached) and inspect traceback backrefs.
1 parent 73a9cec commit 88a5489

3 files changed

Lines changed: 33 additions & 2 deletions

File tree

graalpython/com.oracle.graal.python.test/src/tests/test_frame_tests.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,24 @@ def foo():
238238
assert e.__traceback__.tb_next.tb_frame.f_back.f_code == test_backref_from_traceback.__code__
239239

240240

241+
def test_backref_from_traceback_after_cached_transition():
242+
def bar(should_raise):
243+
if should_raise:
244+
raise RuntimeError
245+
246+
def foo(should_raise):
247+
bar(should_raise)
248+
249+
for _ in range(64): # we probably do not execute 64-times in uncached
250+
foo(False)
251+
252+
try:
253+
foo(True)
254+
except Exception as e:
255+
assert e.__traceback__.tb_next.tb_next.tb_frame.f_back.f_code == foo.__code__
256+
assert e.__traceback__.tb_next.tb_frame.f_back.f_code == test_backref_from_traceback_after_cached_transition.__code__
257+
258+
241259
def test_frame_from_another_thread():
242260
import sys, threading
243261
event1 = threading.Event()

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ public static void doExit(VirtualFrame frame, AbstractTruffleException ate,
589589
@Bind PBytecodeDSLRootNode root,
590590
@Bind BytecodeNode location) {
591591
if (ate instanceof PException pe) {
592-
pe.notifyAddedTracebackFrame(!root.isInternal());
592+
pe.notifyAddedTracebackFrame(frame, !root.isInternal());
593593
}
594594
// We cannot use instrumentation for exceptional exit
595595
if (root.needsTraceAndProfileInstrumentation()) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/exception/PException.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,18 @@ private void markFrameEscaped() {
363363
}
364364
}
365365

366+
private void markFrameEscaped(Frame frame) {
367+
if (frame != null) {
368+
PFrame.Reference currentFrameInfo = PArguments.getCurrentFrameInfo(frame);
369+
if (currentFrameInfo != null) {
370+
currentFrameInfo.markAsEscaped();
371+
if (escapedFrameThread == null) {
372+
escapedFrameThread = Thread.currentThread();
373+
}
374+
}
375+
}
376+
}
377+
366378
public Thread getEscapedFrameThread() {
367379
return escapedFrameThread;
368380
}
@@ -401,9 +413,10 @@ public int getTracebackFrameCount() {
401413
return tracebackFrameCount;
402414
}
403415

404-
public void notifyAddedTracebackFrame(boolean visible) {
416+
public void notifyAddedTracebackFrame(Frame frame, boolean visible) {
405417
if (visible) {
406418
tracebackFrameCount++;
419+
markFrameEscaped(frame);
407420
}
408421
}
409422

0 commit comments

Comments
 (0)