Skip to content

Commit 6b1fdfd

Browse files
committed
Fix inspecting frames from other threads
1 parent a97d544 commit 6b1fdfd

File tree

4 files changed

+117
-14
lines changed

4 files changed

+117
-14
lines changed

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,3 +246,35 @@ def test_clearing_globals():
246246
junk = globals().clear()
247247
assert "foo" not in globals().keys()
248248
assert "junk" in globals().keys()
249+
250+
251+
def test_frame_from_another_thread():
252+
import sys, threading
253+
event1 = threading.Event()
254+
event2 = threading.Event()
255+
event3 = threading.Event()
256+
event4 = threading.Event()
257+
frame = None
258+
def target():
259+
# Mind the line numbers
260+
nonlocal frame
261+
frame = sys._getframe()
262+
a = 1
263+
event1.set()
264+
event2.wait(timeout=60)
265+
b = 2
266+
event3.set()
267+
event4.wait(timeout=60)
268+
thread = threading.Thread(target=target)
269+
thread.start()
270+
event1.wait(timeout=60)
271+
firstlineno = target.__code__.co_firstlineno
272+
assert 5 <= frame.f_lineno - firstlineno <= 6
273+
assert frame.f_locals['a'] == 1
274+
assert 'b' not in frame.f_locals
275+
event2.set()
276+
event3.wait(timeout=60)
277+
assert 8 <= frame.f_lineno - firstlineno <= 9
278+
assert frame.f_locals['b'] == 2
279+
event4.set()
280+
thread.join(timeout=60)

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/frame/PFrame.java

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ public final class PFrame extends PythonBuiltinObject {
7777
private Object localsDict;
7878
private PythonObject globals;
7979
private final Reference virtualFrameInfo;
80+
private final Thread thread;
8081
/**
8182
* For the manual bytecode interpreter the location can be the {@link PBytecodeRootNode} itself,
8283
* but for the Bytecode DSL interpreter, the location must be an AST node connected to the
@@ -107,7 +108,7 @@ public final class PFrame extends PythonBuiltinObject {
107108

108109
/**
109110
* The last {@link CallerFlags} that were used the last time the frame was synced or passed down
110-
* to a callee. See {@link #isStale(VirtualFrame, int)} for more details.
111+
* to a callee. See {@link #needsRefresh(VirtualFrame, int)} for more details.
111112
*/
112113
private int lastCallerFlags;
113114

@@ -208,6 +209,7 @@ public PFrame(PythonLanguage lang, Reference virtualFrameInfo, Node location, bo
208209
// Mark everything as current for now. MaterializeFrameNode will set lastCallerFlags to a
209210
// narrower value if needed
210211
this.lastCallerFlags = CallerFlags.ALL_FRAME_FLAGS;
212+
this.thread = Thread.currentThread();
211213
}
212214

213215
public PFrame(PythonLanguage lang, @SuppressWarnings("unused") Object threadState, PCode code, PythonObject globals, Object localsDict) {
@@ -223,6 +225,7 @@ public PFrame(PythonLanguage lang, @SuppressWarnings("unused") Object threadStat
223225
this.localsDict = localsDict;
224226
// This is a synthetic frame, there will be no sync, mark everything as current
225227
this.lastCallerFlags = CallerFlags.ALL_FRAME_FLAGS;
228+
this.thread = null;
226229
}
227230

228231
/**
@@ -252,18 +255,27 @@ public void setLocals(MaterializedFrame locals) {
252255
* @param callerFlags Specifies which fields should be checked for needing sync. It should be
253256
* flags from {@link CallerFlags}.
254257
*/
255-
public boolean isStale(VirtualFrame frame, int callerFlags) {
256-
if (hasCustomLocals) {
257-
// Custom locals don't need locals sync
258-
callerFlags &= ~CallerFlags.NEEDS_LOCALS;
259-
}
260-
if (CallerFlags.needsLocals(callerFlags) && locals == null) {
258+
public boolean needsRefresh(VirtualFrame frame, int callerFlags) {
259+
if (outdatedCallerFlags(callerFlags)) {
261260
return true;
262261
}
263262
if (CallerFlags.needsLocals(callerFlags) || CallerFlags.needsLasti(callerFlags)) {
264263
if (frame != null && PArguments.getCurrentFrameInfo(frame) == getRef()) {
265264
return true;
266265
}
266+
// Frames from other threads need to go through slow path
267+
return thread != null && thread != Thread.currentThread();
268+
}
269+
return false;
270+
}
271+
272+
public boolean outdatedCallerFlags(int callerFlags) {
273+
if (hasCustomLocals) {
274+
// Custom locals don't need locals sync
275+
callerFlags &= ~CallerFlags.NEEDS_LOCALS;
276+
}
277+
if (CallerFlags.needsLocals(callerFlags) && locals == null) {
278+
return true;
267279
}
268280
return (callerFlags & lastCallerFlags) != callerFlags;
269281
}
@@ -291,6 +303,10 @@ public PFrame.Reference getRef() {
291303
return virtualFrameInfo;
292304
}
293305

306+
public Thread getThread() {
307+
return thread;
308+
}
309+
294310
public PFrame.Reference getBackref() {
295311
return backref;
296312
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,10 @@ static Object doLoop(VirtualFrame frame, Node inliningTarget, PFrame pyFrame, bo
102102
@Cached InlinedBranchProfile create,
103103
@Cached(inline = false) CopyLocalsToDict copyLocalsToDict,
104104
@Cached ReadFrameNode readFrameNode) {
105-
if (!freshFrame && pyFrame.isStale(frame, CallerFlags.NEEDS_LOCALS)) {
105+
if (!freshFrame && pyFrame.needsRefresh(frame, CallerFlags.NEEDS_LOCALS)) {
106106
pyFrame = readFrameNode.refreshFrame(frame, pyFrame.getRef(), CallerFlags.NEEDS_LOCALS);
107107
}
108-
assert !pyFrame.isStale(null, CallerFlags.NEEDS_LOCALS);
108+
assert !pyFrame.outdatedCallerFlags(CallerFlags.NEEDS_LOCALS);
109109
MaterializedFrame locals = pyFrame.getLocals();
110110
// It doesn't have custom locals, so it has to be a builtin dict or null
111111
PDict localsDict = (PDict) pyFrame.getLocalsDict();

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

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,24 +40,37 @@
4040
*/
4141
package com.oracle.graal.python.nodes.frame;
4242

43+
import static com.oracle.graal.python.util.PythonUtils.tsLiteral;
44+
4345
import java.util.Objects;
46+
import java.util.concurrent.CancellationException;
47+
import java.util.concurrent.ExecutionException;
48+
import java.util.concurrent.Future;
49+
import java.util.concurrent.TimeUnit;
50+
import java.util.concurrent.TimeoutException;
4451

52+
import com.oracle.graal.python.PythonLanguage;
53+
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
4554
import com.oracle.graal.python.builtins.objects.frame.PFrame;
4655
import com.oracle.graal.python.builtins.objects.function.PArguments;
4756
import com.oracle.graal.python.builtins.objects.generator.PGenerator;
57+
import com.oracle.graal.python.nodes.PRaiseNode;
4858
import com.oracle.graal.python.nodes.PRootNode;
4959
import com.oracle.graal.python.nodes.bytecode.PBytecodeGeneratorRootNode;
5060
import com.oracle.graal.python.nodes.bytecode.PBytecodeRootNode;
5161
import com.oracle.graal.python.nodes.bytecode_dsl.PBytecodeDSLRootNode;
5262
import com.oracle.graal.python.runtime.CallerFlags;
63+
import com.oracle.graal.python.runtime.GilNode;
5364
import com.oracle.graal.python.runtime.IndirectCallData;
5465
import com.oracle.graal.python.runtime.IndirectCallData.BoundaryCallData;
5566
import com.oracle.graal.python.runtime.PythonContext;
5667
import com.oracle.graal.python.runtime.PythonOptions;
5768
import com.oracle.truffle.api.CompilerDirectives;
5869
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
5970
import com.oracle.truffle.api.RootCallTarget;
71+
import com.oracle.truffle.api.ThreadLocalAction;
6072
import com.oracle.truffle.api.Truffle;
73+
import com.oracle.truffle.api.TruffleSafepoint;
6174
import com.oracle.truffle.api.bytecode.ContinuationRootNode;
6275
import com.oracle.truffle.api.dsl.Bind;
6376
import com.oracle.truffle.api.dsl.Cached;
@@ -144,7 +157,7 @@ public final PFrame refreshFrame(VirtualFrame frame, PFrame.Reference reference,
144157
}
145158

146159
public final PFrame ensureFresh(VirtualFrame frame, PFrame pFrame, int callerFlags) {
147-
if (pFrame.isStale(frame, callerFlags)) {
160+
if (pFrame.needsRefresh(frame, callerFlags)) {
148161
return refreshFrame(frame, pFrame.getRef(), callerFlags);
149162
}
150163
return pFrame;
@@ -166,9 +179,11 @@ PFrame read(VirtualFrame frame, PFrame.Reference startFrameInfo, FrameInstance.F
166179
@Cached MaterializeFrameNode materializeFrameNode,
167180
@Cached InlinedBranchProfile stackWalkProfile1,
168181
@Cached InlinedBranchProfile stackWalkProfile2) {
182+
PFrame.Reference executingFrameInfo = frame != null ? PArguments.getCurrentFrameInfo(frame) : null;
169183
if (startFrameInfo == null) {
170184
PythonContext context = PythonContext.get(this);
171185
startFrameInfo = context.peekTopFrameInfo(context.getLanguage(this));
186+
executingFrameInfo = startFrameInfo;
172187
}
173188
int i = 0;
174189
PFrame.Reference curFrameInfo = startFrameInfo;
@@ -182,10 +197,10 @@ PFrame read(VirtualFrame frame, PFrame.Reference startFrameInfo, FrameInstance.F
182197
if (i == level) {
183198
// We found the right reference
184199
// Maybe it's for the frame we're in?
185-
if (frame != null && PArguments.getCurrentFrameInfo(frame) == curFrameInfo) {
200+
if (curFrameInfo == executingFrameInfo) {
186201
return materializeFrameNode.execute(this, false, CallerFlags.needsLocals(callerFlags), frame);
187202
}
188-
if (curFrameInfo.getPyFrame() != null && !curFrameInfo.getPyFrame().isStale(null, callerFlags)) {
203+
if (curFrameInfo.getPyFrame() != null && !curFrameInfo.getPyFrame().needsRefresh(null, callerFlags)) {
189204
return curFrameInfo.getPyFrame();
190205
}
191206
// We don't have the frame for the reference, fall back to the stack walk
@@ -215,7 +230,7 @@ PFrame read(VirtualFrame frame, PFrame.Reference startFrameInfo, FrameInstance.F
215230
* It is necessary to continue from where we stopped with the backref walk because the
216231
* original starting frame might not be on stack anymore
217232
*/
218-
return readFromStackWalk(curFrameInfo, frameAccess, selector, level - i, callerFlags, materializeFrameNode);
233+
return readSlowPath(curFrameInfo, frameAccess, selector, level - i, callerFlags, materializeFrameNode);
219234
}
220235

221236
private static PFrame.Reference getBackref(PFrame.Reference reference) {
@@ -226,9 +241,49 @@ private static PFrame.Reference getBackref(PFrame.Reference reference) {
226241
}
227242

228243
@TruffleBoundary
229-
private PFrame readFromStackWalk(PFrame.Reference startFrameInfo, FrameInstance.FrameAccess frameAccess, FrameSelector selector, int level, int callerFlags,
244+
@SuppressWarnings("try")
245+
private PFrame readSlowPath(PFrame.Reference startFrameInfo, FrameInstance.FrameAccess frameAccess, FrameSelector selector, int level, int callerFlags,
230246
MaterializeFrameNode materializeFrameNode) {
247+
if (level == 0 && startFrameInfo != null && startFrameInfo.getPyFrame() != null && !selector.skip(startFrameInfo.getRootNode()) && startFrameInfo.getPyFrame().getThread() != null &&
248+
startFrameInfo.getPyFrame().getThread() != Thread.currentThread()) {
249+
// We have the frame we're looking for, but it's on another thread
250+
Thread thread = startFrameInfo.getPyFrame().getThread();
251+
if (thread.isAlive()) {
252+
try (var gil = GilNode.uncachedRelease()) {
253+
// Schedule a safepoint action on that thread
254+
Future<Void> future = PythonContext.get(null).getEnv().submitThreadLocal(new Thread[]{thread}, new ThreadLocalAction(true, false) {
255+
@Override
256+
protected void perform(Access access) {
257+
Node location = access.getLocation();
258+
if (location instanceof PBytecodeDSLRootNode) {
259+
// See AsyncPythonAction#execute for explanation
260+
location = PythonLanguage.get(null).unavailableSafepointLocation;
261+
}
262+
StackWalkResult result = ReadFrameNode.getFrame(location, startFrameInfo, frameAccess, selector, 0, callerFlags);
263+
processStackWalkResult(materializeFrameNode, callerFlags, result);
264+
}
265+
});
266+
TruffleSafepoint.setBlockedThreadInterruptible(this, voidFuture -> {
267+
try {
268+
voidFuture.get(10, TimeUnit.SECONDS);
269+
} catch (TimeoutException e) {
270+
throw PRaiseNode.raiseStatic(this, PythonBuiltinClassType.RuntimeError, tsLiteral("Failed to interrupt thread " + Thread.currentThread() + " within 10 seconds"));
271+
} catch (CancellationException e) {
272+
// Ignore
273+
} catch (ExecutionException e) {
274+
throw new RuntimeException(e);
275+
}
276+
}, future);
277+
}
278+
}
279+
assert !startFrameInfo.getPyFrame().outdatedCallerFlags(callerFlags);
280+
return startFrameInfo.getPyFrame();
281+
}
231282
StackWalkResult callerFrameResult = getFrame(this, startFrameInfo, frameAccess, selector, level, callerFlags);
283+
return processStackWalkResult(materializeFrameNode, callerFlags, callerFrameResult);
284+
}
285+
286+
private static PFrame processStackWalkResult(MaterializeFrameNode materializeFrameNode, int callerFlags, StackWalkResult callerFrameResult) {
232287
if (callerFrameResult != null) {
233288
Node location = callerFrameResult.callNode;
234289
if (!(callerFrameResult.rootNode instanceof PBytecodeDSLRootNode) && location == null) {

0 commit comments

Comments
 (0)