Skip to content

Commit 8f6c154

Browse files
committed
[GR-73063] Attach raising thread to PException when escaping.
PullRequest: graalpython/4508
2 parents cee77bd + 63d2057 commit 8f6c154

4 files changed

Lines changed: 69 additions & 11 deletions

File tree

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,12 @@
3737
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
3838
# SOFTWARE.
3939
import ast
40+
import concurrent.futures
4041
import linecache
4142
import subprocess
4243
import sys
44+
import traceback
45+
import threading
4346

4447

4548
def assert_raises(err, fn, *args, **kwargs):
@@ -63,6 +66,34 @@ def test_traceback_dir():
6366
assert set(dir(tb)) == {'tb_frame', 'tb_next', 'tb_lasti', 'tb_lineno'}
6467

6568

69+
def test_traceback_frame_from_threadpool_exception_published_before_workitem_exit():
70+
published = threading.Event()
71+
release = threading.Event()
72+
original_set_exception = concurrent.futures.Future.set_exception
73+
74+
def set_exception_and_wait(self, exc):
75+
original_set_exception(self, exc)
76+
published.set()
77+
release.wait(timeout=5)
78+
79+
def target():
80+
raise OSError("published before _WorkItem.run exits")
81+
82+
concurrent.futures.Future.set_exception = set_exception_and_wait
83+
try:
84+
executor = concurrent.futures.ThreadPoolExecutor(max_workers=1)
85+
try:
86+
future = executor.submit(target)
87+
assert published.wait(timeout=60)
88+
exc = future.exception(timeout=0)
89+
traceback.clear_frames(exc.__traceback__)
90+
finally:
91+
release.set()
92+
executor.shutdown(wait=True)
93+
finally:
94+
concurrent.futures.Future.set_exception = original_set_exception
95+
96+
6697
def test_import():
6798
imported = True
6899
try:

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/traceback/TracebackBuiltins.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,13 +270,19 @@ static PFrame doOnStack(VirtualFrame frame, PTraceback tb,
270270
Reference frameInfo = tb.getFrameInfo();
271271
assert frameInfo.isEscaped() : createAssertionMessage("Cannot create traceback for non-escaped frame", frameInfo);
272272

273-
PFrame escapedFrame = readCallerFrame.getFrameForReference(frame, frameInfo, ReadFrameNode.AllPythonFramesSelector.INSTANCE, 0, 0);
273+
PFrame escapedFrame = readCallerFrame.getFrameForReference(frame, frameInfo, ReadFrameNode.AllPythonFramesSelector.INSTANCE, 0, 0, getEscapedFrameThread(tb));
274274
assert escapedFrame != null : createAssertionMessage("Failed to find escaped frame on stack", frameInfo);
275275

276276
tb.setFrame(escapedFrame);
277277
return escapedFrame;
278278
}
279279

280+
private static Thread getEscapedFrameThread(PTraceback tb) {
281+
LazyTraceback lazyTraceback = tb.getLazyTraceback();
282+
PException exception = lazyTraceback != null ? lazyTraceback.getException() : null;
283+
return exception != null ? exception.getEscapedFrameThread() : null;
284+
}
285+
280286
@TruffleBoundary
281287
private static String createAssertionMessage(String prefix, Reference frameInfo) {
282288
String stackTrace;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadFrameNode.java

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,17 @@ public final PFrame getFrameForReference(Frame frame, PFrame.Reference startFram
171171
}
172172

173173
public final PFrame getFrameForReference(Frame frame, PFrame.Reference startFrameInfo, FrameSelector selector, int level, int callerFlags) {
174-
return execute(frame, startFrameInfo, FrameInstance.FrameAccess.READ_ONLY, selector, level, callerFlags | CallerFlags.NEEDS_PFRAME);
174+
return getFrameForReference(frame, startFrameInfo, selector, level, callerFlags, null);
175175
}
176176

177-
protected abstract PFrame execute(Frame frame, PFrame.Reference startFrameInfo, FrameInstance.FrameAccess frameAccess, FrameSelector selector, int level, int callerFlags);
177+
public final PFrame getFrameForReference(Frame frame, PFrame.Reference startFrameInfo, FrameSelector selector, int level, int callerFlags, Thread frameThread) {
178+
return execute(frame, startFrameInfo, FrameInstance.FrameAccess.READ_ONLY, selector, level, callerFlags | CallerFlags.NEEDS_PFRAME, frameThread);
179+
}
180+
181+
protected abstract PFrame execute(Frame frame, PFrame.Reference startFrameInfo, FrameInstance.FrameAccess frameAccess, FrameSelector selector, int level, int callerFlags, Thread frameThread);
178182

179183
@Specialization
180-
PFrame read(VirtualFrame frame, PFrame.Reference startFrameInfo, FrameInstance.FrameAccess frameAccess, FrameSelector selector, int level, int callerFlags,
184+
PFrame read(VirtualFrame frame, PFrame.Reference startFrameInfo, FrameInstance.FrameAccess frameAccess, FrameSelector selector, int level, int callerFlags, Thread frameThread,
181185
@Bind Node inliningTarget,
182186
@Cached MaterializeFrameNode materializeFrameNode,
183187
@Cached InlinedBranchProfile stackWalkProfile1,
@@ -233,17 +237,16 @@ PFrame read(VirtualFrame frame, PFrame.Reference startFrameInfo, FrameInstance.F
233237
* It is necessary to continue from where we stopped with the backref walk because the
234238
* original starting frame might not be on stack anymore
235239
*/
236-
return readSlowPath(curFrameInfo, frameAccess, selector, level - i, callerFlags, materializeFrameNode);
240+
return readSlowPath(curFrameInfo, frameAccess, selector, level - i, callerFlags, frameThread, materializeFrameNode);
237241
}
238242

239243
@TruffleBoundary
240244
@SuppressWarnings("try")
241-
private PFrame readSlowPath(PFrame.Reference startFrameInfo, FrameInstance.FrameAccess frameAccess, FrameSelector selector, int level, int callerFlags,
245+
private PFrame readSlowPath(PFrame.Reference startFrameInfo, FrameInstance.FrameAccess frameAccess, FrameSelector selector, int level, int callerFlags, Thread frameThread,
242246
MaterializeFrameNode materializeFrameNode) {
243-
if (level == 0 && startFrameInfo != null && startFrameInfo.getPyFrame() != null && !selector.skip(startFrameInfo.getRootNode()) && startFrameInfo.getPyFrame().getThread() != null &&
244-
startFrameInfo.getPyFrame().getThread() != Thread.currentThread()) {
247+
Thread thread = getFrameThread(startFrameInfo, frameThread);
248+
if (level == 0 && startFrameInfo != null && !selector.skip(startFrameInfo.getRootNode()) && thread != null && thread != Thread.currentThread()) {
245249
// We have the frame we're looking for, but it's on another thread
246-
Thread thread = startFrameInfo.getPyFrame().getThread();
247250
if (thread.isAlive()) {
248251
try (var gil = GilNode.uncachedRelease()) {
249252
// Schedule a safepoint action on that thread
@@ -272,13 +275,23 @@ protected void perform(Access access) {
272275
}, future);
273276
}
274277
}
275-
assert !startFrameInfo.getPyFrame().outdatedCallerFlags(callerFlags);
276-
return startFrameInfo.getPyFrame();
278+
PFrame pyFrame = startFrameInfo.getPyFrame();
279+
if (pyFrame != null) {
280+
assert !pyFrame.outdatedCallerFlags(callerFlags);
281+
return pyFrame;
282+
}
277283
}
278284
StackWalkResult callerFrameResult = getFrame(this, startFrameInfo, frameAccess, selector, level, callerFlags);
279285
return processStackWalkResult(materializeFrameNode, callerFlags, callerFrameResult);
280286
}
281287

288+
private static Thread getFrameThread(PFrame.Reference startFrameInfo, Thread frameThread) {
289+
if (startFrameInfo != null && startFrameInfo.getPyFrame() != null) {
290+
return startFrameInfo.getPyFrame().getThread();
291+
}
292+
return frameThread;
293+
}
294+
282295
private static PFrame processStackWalkResult(MaterializeFrameNode materializeFrameNode, int callerFlags, StackWalkResult callerFrameResult) {
283296
if (callerFrameResult != null) {
284297
Node location = callerFrameResult.callNode;

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ public final class PException extends AbstractTruffleException {
101101
private String message = null;
102102
final transient Object pythonException;
103103
private transient PFrame.Reference frameInfo;
104+
private transient Thread escapedFrameThread;
104105

105106
/**
106107
* Root node that caught this exception object. This node is a manual bytecode or Bytecode DSL
@@ -383,9 +384,16 @@ private void markFrameEscaped() {
383384
// shouldn't leak to the traceback
384385
if (this.frameInfo != null) {
385386
this.frameInfo.markAsEscaped();
387+
if (escapedFrameThread == null) {
388+
escapedFrameThread = Thread.currentThread();
389+
}
386390
}
387391
}
388392

393+
public Thread getEscapedFrameThread() {
394+
return escapedFrameThread;
395+
}
396+
389397
/**
390398
* Get the python exception while ensuring that the traceback frame is marked as escaped
391399
*/

0 commit comments

Comments
 (0)