Skip to content

Commit 3e5017c

Browse files
javeleonjaveleon
committed
[GR-62994] Consolidate logic for JDWP thread enters/exit of the truffle context.
PullRequest: graal/20944
2 parents 1ec3a06 + 635074f commit 3e5017c

File tree

4 files changed

+57
-85
lines changed

4 files changed

+57
-85
lines changed

‎espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerConnection.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ public void run() {
115115

116116
private static class JDWPReceiver implements Runnable {
117117

118+
private static final Object NOT_ENTERED_MARKER = new Object();
118119
private DebuggerController.SetupState setupState;
119120
private final DebuggerController controller;
120121
private RequestedJDWPEvents requestedJDWPEvents;
@@ -199,11 +200,17 @@ public void run() {
199200
setupState = null;
200201
latch.countDown();
201202
}
202-
// Now, begin processing packets when they start to flow from the debugger
203+
// Now, begin processing packets when they start to flow from the debugger.
204+
// Make sure this thread is entered in the context
203205
try {
204206
while (!Thread.currentThread().isInterrupted() && !controller.isClosing()) {
207+
Object previous = NOT_ENTERED_MARKER;
205208
try {
206-
processPacket(Packet.fromByteArray(debuggerConnection.connection.readPacket()));
209+
// get the packet outside the Truffle context, because it's a blocking IO
210+
// operation
211+
Packet packet = Packet.fromByteArray(debuggerConnection.connection.readPacket());
212+
previous = controller.enterTruffleContext();
213+
processPacket(packet);
207214
} catch (IOException e) {
208215
if (!debuggerConnection.isOpen()) {
209216
// when the socket is closed, we're done
@@ -214,6 +221,10 @@ public void run() {
214221
}
215222
} catch (ConnectionClosedException e) {
216223
break;
224+
} finally {
225+
if (previous != NOT_ENTERED_MARKER) {
226+
controller.leaveTruffleContext(previous);
227+
}
217228
}
218229
}
219230
} finally {
@@ -718,4 +729,4 @@ void handleReply(Packet packet, CommandResult result) {
718729
}
719730
}
720731
}
721-
}
732+
}

‎espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -706,17 +706,13 @@ public VMEventListener getEventListener() {
706706
}
707707

708708
public Object enterTruffleContext() {
709-
if (truffleContext != null) {
710-
return truffleContext.enter(null);
711-
}
712-
return null;
709+
assert truffleContext != null;
710+
return truffleContext.enter(null);
713711
}
714712

715713
public void leaveTruffleContext(Object previous) {
716-
if (truffleContext != null) {
717-
// pass null as previous since we know the jdwp thread only ever enters one context
718-
truffleContext.leave(null, previous);
719-
}
714+
assert truffleContext != null;
715+
truffleContext.leave(null, previous);
720716
}
721717

722718
@Override

‎espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -403,9 +403,7 @@ static CommandResult createReply(Packet packet, DebuggerController controller) {
403403
// ensure redefinition atomicity by suspending all
404404
// guest threads during the redefine transaction
405405
Object[] allGuestThreads = controller.getVisibleGuestThreads();
406-
Object prev = null;
407406
try {
408-
prev = controller.enterTruffleContext();
409407
for (Object guestThread : allGuestThreads) {
410408
controller.suspend(guestThread);
411409
}
@@ -420,7 +418,6 @@ static CommandResult createReply(Packet packet, DebuggerController controller) {
420418
for (Object guestThread : allGuestThreads) {
421419
controller.resume(guestThread);
422420
}
423-
controller.leaveTruffleContext(prev);
424421
}
425422
return new CommandResult(reply);
426423
}

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/JDWPContextImpl.java

Lines changed: 39 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -275,20 +275,18 @@ public Object allocateInstance(KlassRef klass) {
275275

276276
@Override
277277
public void steppingInProgress(Thread t, boolean value) {
278-
performInContext(() -> {
279-
context.getLanguage().getThreadLocalStateFor(t).setSteppingInProgress(value);
280-
return null;
281-
});
278+
context.getLanguage().getThreadLocalStateFor(t).setSteppingInProgress(value);
282279
}
283280

284281
@Override
285282
public boolean isSteppingInProgress(Thread t) {
286-
Object previous = null;
287-
try {
288-
previous = controller.enterTruffleContext();
289-
return context.getLanguage().getThreadLocalStateFor(t).isSteppingInProgress();
290-
} finally {
291-
controller.leaveTruffleContext(previous);
283+
EspressoThreadLocalState state = context.getLanguage().getThreadLocalStateFor(t);
284+
// Here, the thread local state can be null for threads having been unregistered already.
285+
// This is OK, and we can safely return false in such cases.
286+
if (state != null) {
287+
return state.isSteppingInProgress();
288+
} else {
289+
return false;
292290
}
293291
}
294292

@@ -304,13 +302,13 @@ public Object[] getAllGuestThreads() {
304302
}
305303
result.add(activeThread);
306304
}
307-
return result.toArray(new StaticObject[result.size()]);
305+
return result.toArray(StaticObject.EMPTY_ARRAY);
308306
}
309307

310308
@Override
311309
public String getStringValue(Object object) {
312310
if (object instanceof StaticObject staticObject) {
313-
return performInContext(() -> (String) UNCACHED.toDisplayString(staticObject, false));
311+
return (String) UNCACHED.toDisplayString(staticObject, false);
314312
}
315313
return object.toString();
316314
}
@@ -422,13 +420,12 @@ public int getArrayLength(Object array) {
422420
StaticObject staticObject = (StaticObject) array;
423421
EspressoLanguage language = context.getLanguage();
424422
if (staticObject.isForeignObject()) {
425-
long arrayLength = performInContext(() -> {
426-
try {
427-
return UNCACHED.getArraySize(staticObject.rawForeignObject(language));
428-
} catch (UnsupportedMessageException e) {
429-
return (long) -1;
430-
}
431-
});
423+
long arrayLength;
424+
try {
425+
arrayLength = UNCACHED.getArraySize(staticObject.rawForeignObject(language));
426+
} catch (UnsupportedMessageException e) {
427+
return -1;
428+
}
432429
if (arrayLength > Integer.MAX_VALUE) {
433430
return -1;
434431
}
@@ -486,19 +483,17 @@ public Object getArrayValue(Object array, int index) {
486483
Klass componentType = ((ArrayKlass) arrayRef.getKlass()).getComponentType();
487484
Meta meta = componentType.getMeta();
488485
if (arrayRef.isForeignObject()) {
489-
return performInContext(() -> {
490-
Object value = null;
491-
try {
492-
value = UNCACHED.readArrayElement(arrayRef.rawForeignObject(arrayRef.getKlass().getLanguage()), index);
493-
return ToEspressoNode.getUncachedToEspresso(componentType, meta).execute(value);
494-
} catch (UnsupportedMessageException e) {
495-
throw EspressoError.shouldNotReachHere("readArrayElement on a non-array foreign object", e);
496-
} catch (InvalidArrayIndexException e) {
497-
throw meta.throwExceptionWithMessage(meta.java_lang_ArrayIndexOutOfBoundsException, e.getMessage());
498-
} catch (UnsupportedTypeException e) {
499-
throw meta.throwExceptionWithMessage(meta.java_lang_ClassCastException, "%s cannot be cast to %s", value, componentType.getTypeAsString());
500-
}
501-
});
486+
Object value = null;
487+
try {
488+
value = UNCACHED.readArrayElement(arrayRef.rawForeignObject(arrayRef.getKlass().getLanguage()), index);
489+
return ToEspressoNode.getUncachedToEspresso(componentType, meta).execute(value);
490+
} catch (UnsupportedMessageException e) {
491+
throw EspressoError.shouldNotReachHere("readArrayElement on a non-array foreign object", e);
492+
} catch (InvalidArrayIndexException e) {
493+
throw meta.throwExceptionWithMessage(meta.java_lang_ArrayIndexOutOfBoundsException, e.getMessage());
494+
} catch (UnsupportedTypeException e) {
495+
throw meta.throwExceptionWithMessage(meta.java_lang_ClassCastException, "%s cannot be cast to %s", value, componentType.getTypeAsString());
496+
}
502497
} else if (componentType.isPrimitive()) {
503498
// primitive array type needs wrapping
504499
Object boxedArray = getUnboxedArray(array);
@@ -520,18 +515,15 @@ public void setArrayValue(Object array, int index, Object value) {
520515
} else {
521516
unWrappedValue = value;
522517
}
523-
performInContext(() -> {
524-
try {
525-
UNCACHED.writeArrayElement(arrayRef.rawForeignObject(arrayRef.getKlass().getLanguage()), index, unWrappedValue);
526-
return null;
527-
} catch (UnsupportedMessageException e) {
528-
throw EspressoError.shouldNotReachHere("writeArrayElement on a non-array foreign object", e);
529-
} catch (UnsupportedTypeException e) {
530-
throw meta.throwExceptionWithMessage(meta.java_lang_ClassCastException, "%s cannot be cast to %s", value, componentType.getTypeAsString());
531-
} catch (InvalidArrayIndexException e) {
532-
throw meta.throwExceptionWithMessage(meta.java_lang_ArrayIndexOutOfBoundsException, e.getMessage());
533-
}
534-
});
518+
try {
519+
UNCACHED.writeArrayElement(arrayRef.rawForeignObject(arrayRef.getKlass().getLanguage()), index, unWrappedValue);
520+
} catch (UnsupportedMessageException e) {
521+
throw EspressoError.shouldNotReachHere("writeArrayElement on a non-array foreign object", e);
522+
} catch (UnsupportedTypeException e) {
523+
throw meta.throwExceptionWithMessage(meta.java_lang_ClassCastException, "%s cannot be cast to %s", value, componentType.getTypeAsString());
524+
} catch (InvalidArrayIndexException e) {
525+
throw meta.throwExceptionWithMessage(meta.java_lang_ArrayIndexOutOfBoundsException, e.getMessage());
526+
}
535527
} else if (componentType.isPrimitive()) {
536528
// primitive array type needs wrapping
537529
Object boxedArray = getUnboxedArray(array);
@@ -618,18 +610,12 @@ public boolean isInstanceOf(Object object, KlassRef klass) {
618610

619611
@Override
620612
public void stopThread(Object guestThread, Object guestThrowable) {
621-
performInContext(() -> {
622-
context.getThreadAccess().stop((StaticObject) guestThread, (StaticObject) guestThrowable);
623-
return null;
624-
});
613+
context.getThreadAccess().stop((StaticObject) guestThread, (StaticObject) guestThrowable);
625614
}
626615

627616
@Override
628617
public void interruptThread(Object thread) {
629-
performInContext(() -> {
630-
context.interruptThread((StaticObject) thread);
631-
return null;
632-
});
618+
context.interruptThread((StaticObject) thread);
633619
}
634620

635621
@Override
@@ -639,10 +625,7 @@ public boolean systemExitImplemented() {
639625

640626
@Override
641627
public void exit(int exitCode) {
642-
performInContext(() -> {
643-
context.truffleExit(null, exitCode);
644-
return null;
645-
});
628+
context.truffleExit(null, exitCode);
646629
}
647630

648631
@Override
@@ -842,19 +825,4 @@ public ModuleRef[] getAllModulesRefs() {
842825
public synchronized int redefineClasses(List<RedefineInfo> redefineInfos) {
843826
return context.getClassRedefinition().redefineClasses(redefineInfos, false, true);
844827
}
845-
846-
private <R> R performInContext(InContextAction<R> action) {
847-
Object previous = null;
848-
try {
849-
previous = controller.enterTruffleContext();
850-
return action.call();
851-
} finally {
852-
controller.leaveTruffleContext(previous);
853-
}
854-
}
855-
856-
@FunctionalInterface
857-
private interface InContextAction<R> {
858-
R call();
859-
}
860828
}

0 commit comments

Comments
 (0)