Skip to content

Commit 9f74c9f

Browse files
javeleonjaveleon
committed
[GR-39436] Improve resume and suspend logic in Espresso jdwp.
PullRequest: graal/14387
2 parents d285318 + 2e32977 commit 9f74c9f

File tree

3 files changed

+45
-36
lines changed

3 files changed

+45
-36
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public void doProcessCommands(boolean suspend, Collection<Thread> activeThreads,
6868
return;
6969
}
7070
// only a JDWP resume/resumeAll command can resume this thread
71-
controller.suspend(null, context.asGuestThread(Thread.currentThread()), SuspendStrategy.EVENT_THREAD, Collections.singletonList(job), null, false);
71+
controller.suspend(context.asGuestThread(Thread.currentThread()), SuspendStrategy.EVENT_THREAD, Collections.singletonList(job), true);
7272
}
7373
}
7474

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

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -445,11 +445,11 @@ public void immediateSuspend(Object eventThread, byte suspendPolicy, Callable<Vo
445445
}
446446
}
447447
// immediately suspend the event thread
448-
suspend(null, eventThread, SuspendStrategy.EVENT_THREAD, Collections.singletonList(callBack), null, false);
448+
suspend(eventThread, SuspendStrategy.EVENT_THREAD, Collections.singletonList(callBack), true);
449449
break;
450450
case SuspendStrategy.EVENT_THREAD:
451451
// immediately suspend the event thread
452-
suspend(null, eventThread, SuspendStrategy.EVENT_THREAD, Collections.singletonList(callBack), null, false);
452+
suspend(eventThread, SuspendStrategy.EVENT_THREAD, Collections.singletonList(callBack), true);
453453
break;
454454
}
455455
}
@@ -485,7 +485,7 @@ public void disposeDebugger(boolean prepareReconnect) {
485485
// to a dead VM from a JDWP client point of view
486486
if (eventListener.vmDied()) {
487487
// we're asked to suspend
488-
suspend(null, context.asGuestThread(Thread.currentThread()), SuspendStrategy.EVENT_THREAD, Collections.emptyList(), null, false);
488+
suspend(context.asGuestThread(Thread.currentThread()), SuspendStrategy.EVENT_THREAD, Collections.emptyList(), true);
489489
}
490490
}
491491
// Creating a new thread, because the reset method
@@ -558,7 +558,7 @@ public void onLanguageContextInitialized(TruffleContext con, @SuppressWarnings("
558558
truffleContext = con;
559559
}
560560

561-
public void suspend(CallFrame currentFrame, Object thread, byte suspendPolicy, List<Callable<Void>> jobs, SteppingInfo steppingInfo, boolean breakpointHit) {
561+
public void suspend(Object thread, byte suspendPolicy, List<Callable<Void>> jobs, boolean forceSuspend) {
562562
fine(() -> "suspending from callback in thread: " + getThreadName(thread));
563563

564564
// before sending any events to debugger, make sure to mark
@@ -575,10 +575,7 @@ public void suspend(CallFrame currentFrame, Object thread, byte suspendPolicy, L
575575
break;
576576
case SuspendStrategy.EVENT_THREAD:
577577
fine(() -> "Suspend EVENT_THREAD");
578-
579-
threadSuspension.suspendThread(thread);
580-
runJobs(jobs);
581-
suspendEventThread(currentFrame, thread, steppingInfo, breakpointHit);
578+
suspendEventThread(thread, forceSuspend, jobs);
582579
break;
583580
case SuspendStrategy.ALL:
584581
fine(() -> "Suspend ALL");
@@ -593,15 +590,10 @@ public void run() {
593590
DebuggerController.this.suspend(activeThread);
594591
}
595592
}
596-
// send any breakpoint events here, since now all threads that are
597-
// expected to be suspended
598-
// have increased suspension count
599-
runJobs(jobs);
600593
}
601594
});
602-
threadSuspension.suspendThread(thread);
603595
suspendThread.start();
604-
suspendEventThread(currentFrame, thread, steppingInfo, breakpointHit);
596+
suspendEventThread(thread, forceSuspend, jobs);
605597
break;
606598
}
607599
}
@@ -616,29 +608,30 @@ private static void runJobs(List<Callable<Void>> jobs) {
616608
}
617609
}
618610

619-
private void suspendEventThread(CallFrame currentFrame, Object thread, SteppingInfo info, boolean breakpointHit) {
611+
private void suspendEventThread(Object thread, boolean forceSuspend, List<Callable<Void>> jobs) {
620612
fine(() -> "Suspending event thread: " + getThreadName(thread) + " with new suspension count: " + threadSuspension.getSuspensionCount(thread));
621-
622-
// if during stepping, send a step completed event back to the debugger
623-
if (info != null && !breakpointHit) {
624-
eventListener.stepCompleted(info, currentFrame);
625-
}
626-
627-
// no reason to hold a hard suspension status, since now
628-
// we have the actual suspension status and suspended information
629-
threadSuspension.removeHardSuspendedThread(thread);
630-
631-
lockThread(thread);
613+
lockThread(thread, forceSuspend, true, jobs);
632614
}
633615

634-
private void lockThread(Object thread) {
616+
private void lockThread(Object thread, boolean forceSuspend, boolean isFirstCall, List<Callable<Void>> jobs) {
635617
SimpleLock lock = getSuspendLock(thread);
636618
// in case a thread job is already posted on this thread
637-
checkThreadJobsAndRun(thread);
619+
checkThreadJobsAndRun(thread, forceSuspend);
638620
synchronized (lock) {
621+
if (!forceSuspend && !threadSuspension.isHardSuspended(thread)) {
622+
// thread was resumed from other command, so don't suspend now
623+
return;
624+
}
639625
try {
626+
if (lock.isLocked() && isFirstCall) {
627+
threadSuspension.suspendThread(thread);
628+
runJobs(jobs);
629+
}
640630
while (lock.isLocked()) {
641631
fine(() -> "lock.wait() for thread: " + getThreadName(thread));
632+
// no reason to hold a hard suspension status, since now
633+
// we have the actual suspension status and suspended information
634+
threadSuspension.removeHardSuspendedThread(thread);
642635
lock.wait();
643636
}
644637
} catch (InterruptedException e) {
@@ -648,12 +641,12 @@ private void lockThread(Object thread) {
648641
}
649642
}
650643

651-
checkThreadJobsAndRun(thread);
644+
checkThreadJobsAndRun(thread, forceSuspend);
652645
getGCPrevention().releaseActiveWhileSuspended(thread);
653646
fine(() -> "lock wakeup for thread: " + getThreadName(thread));
654647
}
655648

656-
private void checkThreadJobsAndRun(Object thread) {
649+
private void checkThreadJobsAndRun(Object thread, boolean forceSuspend) {
657650
if (threadJobs.containsKey(thread)) {
658651
// re-acquire the thread lock after completing
659652
// the job, to avoid the thread resuming.
@@ -685,7 +678,7 @@ private void checkThreadJobsAndRun(Object thread) {
685678
} else {
686679
job.runJob();
687680
}
688-
lockThread(thread);
681+
lockThread(thread, forceSuspend, false, Collections.emptyList());
689682
}
690683
}
691684

@@ -920,6 +913,7 @@ public Void call() {
920913
if (fieldEvent != null) {
921914
FieldBreakpointInfo info = fieldEvent.getInfo();
922915
if (info.isAccessBreakpoint()) {
916+
hit = true;
923917
jobs.add(new Callable<>() {
924918
@Override
925919
public Void call() {
@@ -928,6 +922,7 @@ public Void call() {
928922
}
929923
});
930924
} else if (info.isModificationBreakpoint()) {
925+
hit = true;
931926
jobs.add(new Callable<>() {
932927
@Override
933928
public Void call() {
@@ -940,6 +935,7 @@ public Void call() {
940935
// check if suspended for a method breakpoint
941936
MethodBreakpointEvent methodEvent = methodBreakpointExpected.remove(Thread.currentThread());
942937
if (methodEvent != null) {
938+
hit = true;
943939
jobs.add(new Callable<>() {
944940
@Override
945941
public Void call() {
@@ -948,9 +944,18 @@ public Void call() {
948944
}
949945
});
950946
}
947+
if (steppingInfo != null) {
948+
jobs.add(new Callable<>() {
949+
@Override
950+
public Void call() {
951+
eventListener.stepCompleted(steppingInfo, callFrames[0]);
952+
return null;
953+
}
954+
});
955+
}
951956

952957
// now, suspend the current thread until resumed by e.g. a debugger command
953-
suspend(callFrames[0], currentThread, suspendPolicy, jobs, steppingInfo, hit);
958+
suspend(currentThread, suspendPolicy, jobs, hit || steppingInfo != null);
954959
}
955960

956961
private boolean matchLocation(Pattern[] patterns, CallFrame callFrame) {

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2019, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -22,12 +22,12 @@
2222
*/
2323
package com.oracle.truffle.espresso.jdwp.impl;
2424

25-
import com.oracle.truffle.api.CompilerDirectives;
26-
2725
import java.util.Arrays;
2826
import java.util.HashSet;
2927
import java.util.Set;
3028

29+
import com.oracle.truffle.api.CompilerDirectives;
30+
3131
public final class ThreadSuspension {
3232

3333
private Object[] threads = new Object[0];
@@ -111,4 +111,8 @@ public synchronized void addHardSuspendedThread(Object thread) {
111111
public synchronized void removeHardSuspendedThread(Object thread) {
112112
hardSuspendedThreads.remove(thread);
113113
}
114+
115+
public synchronized boolean isHardSuspended(Object thread) {
116+
return hardSuspendedThreads.contains(thread);
117+
}
114118
}

0 commit comments

Comments
 (0)