diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/Ids.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/Ids.java index 54cc0c352cd4..464b88edf438 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/Ids.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/Ids.java @@ -22,15 +22,17 @@ */ package com.oracle.truffle.espresso.jdwp.api; -import com.oracle.truffle.espresso.jdwp.impl.DebuggerController; - import java.lang.ref.WeakReference; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; +import com.oracle.truffle.espresso.jdwp.impl.DebuggerController; + /** * Class that keeps an ID representation of all entities when communicating with a debugger through * JDWP. Each entity will be assigned a unique ID. Only weak references are kept for entities. @@ -57,6 +59,10 @@ public final class Ids { private DebuggerController controller; + private List pinnedObjects = new ArrayList<>(); + + private volatile boolean pinningState = false; + @SuppressWarnings({"unchecked", "rawtypes"}) public Ids(T nullObject) { this.nullObject = nullObject; @@ -84,8 +90,7 @@ public long getIdAsLong(T object) { } } // check the anonymous inner class map - if (object instanceof KlassRef) { - KlassRef klass = (KlassRef) object; + if (object instanceof KlassRef klass) { Long id = innerClassIDMap.get(klass.getNameAsString()); if (id != null) { // inject new klass under existing ID @@ -155,13 +160,16 @@ private synchronized long generateUniqueId(T object) { expandedArray[objects.length] = new WeakReference<>(object); objects = expandedArray; log(() -> "Generating new ID: " + id + " for object: " + object); - if (object instanceof KlassRef) { - KlassRef klass = (KlassRef) object; + if (object instanceof KlassRef klass) { Matcher matcher = ANON_INNER_CLASS_PATTERN.matcher(klass.getNameAsString()); if (matcher.matches()) { innerClassIDMap.put(klass.getNameAsString(), id); } } + // pin object when VM in suspended state + if (pinningState) { + pinnedObjects.add(object); + } return id; } @@ -203,4 +211,19 @@ private void log(Supplier supplier) { controller.finest(supplier); } } + + public synchronized void pinAll() { + pinningState = true; + for (WeakReference object : objects) { + Object toPin = object.get(); + if (toPin != null) { + pinnedObjects.add(toPin); + } + } + } + + public synchronized void unpinAll() { + pinningState = false; + pinnedObjects.clear(); + } } \ No newline at end of file diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java index b14149b9b1b4..2e0ce619a512 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/DebuggerController.java @@ -75,7 +75,7 @@ public final class DebuggerController implements ContextsListener { private final Map suspendLocks = Collections.synchronizedMap(new HashMap<>()); private final Map suspendedInfos = Collections.synchronizedMap(new HashMap<>()); private final Map commandRequestIds = new HashMap<>(); - private final Map> threadJobs = new HashMap<>(); + private final Map> invokeJobs = new HashMap<>(); private final Map fieldBreakpointExpected = new HashMap<>(); private final Map methodBreakpointExpected = new HashMap<>(); private final Map breakpointInfos = new HashMap<>(); @@ -176,6 +176,10 @@ public JDWPContext getContext() { return context; } + public Ids getIds() { + return ids; + } + public SuspendedInfo getSuspendedInfo(Object thread) { return suspendedInfos.get(thread); } @@ -386,6 +390,7 @@ public Object[] getVisibleGuestThreads() { } void forceResumeAll() { + ids.unpinAll(); for (Object thread : getVisibleGuestThreads()) { boolean resumed = false; SimpleLock suspendLock = getSuspendLock(thread); @@ -398,6 +403,7 @@ void forceResumeAll() { } public void resumeAll() { + ids.unpinAll(); for (Object thread : getVisibleGuestThreads()) { SimpleLock suspendLock = getSuspendLock(thread); synchronized (suspendLock) { @@ -453,6 +459,9 @@ public void immediateSuspend(Object eventThread, byte suspendPolicy, Callable> jobs case SuspendStrategy.ALL: fine(() -> "Suspend ALL"); - Thread suspendThread = new Thread(new Runnable() { - @Override - public void run() { - // suspend other threads - for (Object activeThread : getVisibleGuestThreads()) { - if (activeThread != thread) { - fine(() -> "Request thread suspend for other thread: " + getThreadName(activeThread)); - DebuggerController.this.suspend(activeThread); - } + Thread suspendThread = new Thread(() -> { + // suspend other threads + for (Object activeThread : getVisibleGuestThreads()) { + if (activeThread != thread) { + fine(() -> "Request thread suspend for other thread: " + getThreadName(activeThread)); + DebuggerController.this.suspend(activeThread); } } }); suspendThread.start(); + // pin all objects + ids.pinAll(); suspendEventThread(thread, forceSuspend, jobs); break; } @@ -661,83 +671,60 @@ private static void runJobs(List> jobs) { private void suspendEventThread(Object thread, boolean forceSuspend, List> jobs) { fine(() -> "Suspending event thread: " + getThreadName(thread) + " with new suspension count: " + threadSuspension.getSuspensionCount(thread)); - lockThread(thread, forceSuspend, true, jobs); + lockThread(thread, forceSuspend, jobs); } - private void lockThread(Object thread, boolean forceSuspend, boolean isFirstCall, List> jobs) { + private void lockThread(Object thread, boolean forceSuspend, List> jobs) { SimpleLock lock = getSuspendLock(thread); - // in case a thread job is already posted on this thread - checkThreadJobsAndRun(thread, forceSuspend); synchronized (lock) { if (!forceSuspend && !threadSuspension.isHardSuspended(thread)) { // thread was resumed from other command, so don't suspend now return; } + + if (lock.isLocked()) { + threadSuspension.suspendThread(thread); + runJobs(jobs); + } + } + while (!Thread.currentThread().isInterrupted()) { try { - if (lock.isLocked() && isFirstCall) { - threadSuspension.suspendThread(thread); - runJobs(jobs); - } - while (lock.isLocked()) { - fine(() -> "lock.wait() for thread: " + getThreadName(thread)); + synchronized (lock) { + if (!lock.isLocked()) { + // released from other thread, so break loop + break; + } // no reason to hold a hard suspension status, since now // we have the actual suspension status and suspended information threadSuspension.removeHardSuspendedThread(thread); - lock.wait(); + fine(() -> "lock.wait() for thread: " + getThreadName(thread)); + // Having the thread lock, we can check if an invoke job was posted outside of + // locking, and if so, we postpone blocking the thread until next time around. + if (!invokeJobs.containsKey(thread)) { + lock.wait(); + } } } catch (InterruptedException e) { // the thread was interrupted, so let it run dry // make sure the interrupted flag is set though Thread.currentThread().interrupt(); } + checkInvokeJobsAndRun(thread); } - - checkThreadJobsAndRun(thread, forceSuspend); - getGCPrevention().releaseActiveWhileSuspended(thread); fine(() -> "lock wakeup for thread: " + getThreadName(thread)); } - private void checkThreadJobsAndRun(Object thread, boolean forceSuspend) { - if (threadJobs.containsKey(thread)) { - // re-acquire the thread lock after completing - // the job, to avoid the thread resuming. - SimpleLock suspendLock = getSuspendLock(thread); - synchronized (suspendLock) { - suspendLock.acquire(); - } - // a thread job was posted on this thread - // only wake up to perform the job a go back to sleep - ThreadJob job = threadJobs.remove(thread); - byte suspensionStrategy = job.getSuspensionStrategy(); - - if (suspensionStrategy == SuspendStrategy.ALL) { - Object[] allThreads = getVisibleGuestThreads(); - // resume all threads during invocation of method to avoid potential deadlocks - for (Object activeThread : allThreads) { - if (activeThread != thread) { - resume(activeThread); - } - } - // perform the job on this thread - job.runJob(); - // suspend all other threads after the invocation - for (Object activeThread : allThreads) { - if (activeThread != thread) { - suspend(activeThread); - } - } - } else { - job.runJob(); - } - lockThread(thread, forceSuspend, false, Collections.emptyList()); + private void checkInvokeJobsAndRun(Object thread) { + if (invokeJobs.containsKey(thread)) { + InvokeJob job = invokeJobs.remove(thread); + job.runJob(this); } } - public void postJobForThread(ThreadJob job) { + public void postInvokeJobForThread(InvokeJob job) { SimpleLock lock = getSuspendLock(job.getThread()); synchronized (lock) { - threadJobs.put(job.getThread(), job); - lock.release(); + invokeJobs.put(job.getThread(), job); lock.notifyAll(); } } diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/GCPrevention.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/GCPrevention.java index 2d9547d3b7fe..910066cb9d1f 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/GCPrevention.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/GCPrevention.java @@ -22,8 +22,6 @@ */ package com.oracle.truffle.espresso.jdwp.impl; -import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; public final class GCPrevention { @@ -31,7 +29,6 @@ public final class GCPrevention { // simply hold a strong reference to all objects for which // GC should be disabled private final HashSet prevent = new HashSet<>(); - private final HashMap> activeWhileSuspended = new HashMap<>(); public void disableGC(Object object) { prevent.add(object); @@ -45,13 +42,4 @@ public void clearAll() { prevent.clear(); } - public synchronized void setActiveWhileSuspended(Object guestThread, Object obj) { - activeWhileSuspended.putIfAbsent(guestThread, new ArrayList<>()); - activeWhileSuspended.get(guestThread).add(obj); - } - - public synchronized void releaseActiveWhileSuspended(Object guestThread) { - activeWhileSuspended.remove(guestThread); - } - } diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/ThreadJob.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/InvokeJob.java similarity index 70% rename from espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/ThreadJob.java rename to espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/InvokeJob.java index 7ce260f05ded..e657ef303403 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/ThreadJob.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/InvokeJob.java @@ -24,7 +24,7 @@ import java.util.concurrent.Callable; -public final class ThreadJob { +public final class InvokeJob { private final Object jobLock = new Object(); private final Object thread; @@ -33,11 +33,11 @@ public final class ThreadJob { private boolean resultAvailable; private JobResult result; - public ThreadJob(Object guestThread, Callable task) { + public InvokeJob(Object guestThread, Callable task) { this(guestThread, task, SuspendStrategy.EVENT_THREAD); } - public ThreadJob(Object guestThread, Callable task, byte suspensionStrategy) { + public InvokeJob(Object guestThread, Callable task, byte suspensionStrategy) { this.thread = guestThread; this.callable = task; this.suspensionStrategy = suspensionStrategy; @@ -47,17 +47,33 @@ public Object getThread() { return thread; } - public byte getSuspensionStrategy() { - return suspensionStrategy; - } - - public void runJob() { + public void runJob(DebuggerController controller) { + Object[] visibleGuestThreads = controller.getVisibleGuestThreads(); result = new JobResult<>(); try { + if (suspensionStrategy == SuspendStrategy.ALL) { + controller.getIds().unpinAll(); + // resume all other threads during invocation of method to avoid potential deadlocks + for (Object activeThread : visibleGuestThreads) { + if (activeThread != thread) { + controller.resume(activeThread); + } + } + } + // perform the job on this thread result.setResult(callable.call()); } catch (Throwable e) { result.setException(e); } finally { + if (suspensionStrategy == SuspendStrategy.ALL) { + controller.getIds().pinAll(); + // suspend all other threads after the invocation + for (Object activeThread : visibleGuestThreads) { + if (activeThread != thread) { + controller.suspend(activeThread); + } + } + } resultAvailable = true; synchronized (jobLock) { jobLock.notifyAll(); diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java index 2e799106cb91..00d21093a9dc 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java @@ -1163,8 +1163,8 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D try { // we have to call the method in the correct thread, so post a // Callable to the controller and wait for the result to appear - ThreadJob job = new ThreadJob<>(thread, () -> method.invokeMethodStatic(args), suspensionStrategy); - controller.postJobForThread(job); + InvokeJob job = new InvokeJob<>(thread, () -> method.invokeMethodStatic(args), suspensionStrategy); + controller.postInvokeJobForThread(job); // invocation of a method can cause events with possible thread suspension // to happen, e.g. class prepare events for newly loaded classes @@ -1175,8 +1175,8 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D public void run() { CommandResult commandResult = new CommandResult(reply); try { - ThreadJob.JobResult result = job.getResult(); - writeMethodResult(reply, context, result, thread, controller); + InvokeJob.JobResult result = job.getResult(); + writeMethodResult(reply, context, result); } catch (Throwable t) { reply.errorCode(ErrorCodes.INTERNAL); controller.severe(INVOKE_METHOD.class.getName() + ".createReply", t); @@ -1246,15 +1246,15 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { try { // we have to call the constructor in the correct thread, so post a // Callable to the controller and wait for the result to appear - ThreadJob job = new ThreadJob<>(thread, () -> { + InvokeJob job = new InvokeJob<>(thread, () -> { args[0] = context.allocateInstance(klass); method.invokeMethodSpecial(args); return args[0]; }, suspensionStrategy); - controller.postJobForThread(job); - ThreadJob.JobResult result = job.getResult(); + controller.postInvokeJobForThread(job); + InvokeJob.JobResult result = job.getResult(); - writeMethodResult(reply, context, result, thread, controller); + writeMethodResult(reply, context, result); } catch (Throwable t) { throw new RuntimeException("not able to invoke static method through jdwp", t); } @@ -1350,8 +1350,8 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D try { // we have to call the method in the correct thread, so post a // Callable to the controller and wait for the result to appear - ThreadJob job = new ThreadJob<>(thread, () -> method.invokeMethodStatic(args), suspensionStrategy); - controller.postJobForThread(job); + InvokeJob job = new InvokeJob<>(thread, () -> method.invokeMethodStatic(args), suspensionStrategy); + controller.postInvokeJobForThread(job); // invocation of a method can cause events with possible thread suspension // to happen, e.g. class prepare events for newly loaded classes // to avoid blocking here, we fire up a new thread that will post @@ -1361,8 +1361,8 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D public void run() { CommandResult commandResult = new CommandResult(reply); try { - ThreadJob.JobResult result = job.getResult(); - writeMethodResult(reply, context, result, thread, controller); + InvokeJob.JobResult result = job.getResult(); + writeMethodResult(reply, context, result); } catch (Throwable t) { reply.errorCode(ErrorCodes.INTERNAL); controller.severe(INVOKE_METHOD.class.getName() + "." + "createReply", t); @@ -1587,7 +1587,7 @@ static CommandResult createReply(Packet packet, JDWPContext context) { Object object = context.getIds().fromId((int) objectId); - if (object == context.getNullObject()) { + if (object == null || object == context.getNullObject()) { reply.errorCode(ErrorCodes.INVALID_OBJECT); return new CommandResult(reply); } @@ -1807,7 +1807,7 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D try { // we have to call the method in the correct thread, so post a // Callable to the controller and wait for the result to appear - ThreadJob job = new ThreadJob<>(thread, () -> { + InvokeJob job = new InvokeJob<>(thread, () -> { if (invokeNonvirtual) { return method.invokeMethodNonVirtual(args); } else if (Modifier.isPrivate(method.getModifiers())) { @@ -1818,7 +1818,7 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D return method.invokeMethodVirtual(args); } }, suspensionStrategy); - controller.postJobForThread(job); + controller.postInvokeJobForThread(job); // invocation of a method can cause events with possible thread suspension // to happen, e.g. class prepare events for newly loaded classes // to avoid blocking here, we fire up a new thread that will post @@ -1828,8 +1828,8 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D public void run() { CommandResult commandResult = new CommandResult(reply); try { - ThreadJob.JobResult result = job.getResult(); - writeMethodResult(reply, context, result, thread, controller); + InvokeJob.JobResult result = job.getResult(); + writeMethodResult(reply, context, result); } catch (Throwable t) { reply.errorCode(ErrorCodes.INTERNAL); controller.severe(INVOKE_METHOD.class.getName() + "." + "createReply", t); @@ -2393,11 +2393,11 @@ static CommandResult createReply(Packet packet, DebuggerController controller) { } // make sure owned monitors taken in frame are exited - ThreadJob job = new ThreadJob<>(thread, () -> { + InvokeJob job = new InvokeJob<>(thread, () -> { controller.getContext().clearFrameMonitors(topFrame); return null; }); - controller.postJobForThread(job); + controller.postInvokeJobForThread(job); // don't return here before job completed job.getResult(); @@ -3042,24 +3042,18 @@ public static void writeValue(byte tag, Object value, PacketStream reply, boolea } } - private static void writeMethodResult(PacketStream reply, JDWPContext context, ThreadJob.JobResult result, Object thread, DebuggerController controller) { + private static void writeMethodResult(PacketStream reply, JDWPContext context, InvokeJob.JobResult result) { if (result.getException() != null) { reply.writeByte(TagConstants.OBJECT); reply.writeLong(0); reply.writeByte(TagConstants.OBJECT); Object guestException = context.getGuestException(result.getException()); reply.writeLong(context.getIds().getIdAsLong(guestException)); - if (controller.getThreadSuspension().getSuspensionCount(thread) > 0) { - controller.getGCPrevention().setActiveWhileSuspended(thread, guestException); - } } else { Object value = result.getResult(); if (value != null) { byte tag = context.getTag(value); writeValue(tag, value, reply, true, context); - if (controller.getThreadSuspension().getSuspensionCount(thread) > 0) { - controller.getGCPrevention().setActiveWhileSuspended(thread, value); - } } else { // return value is null reply.writeByte(TagConstants.OBJECT); reply.writeLong(0); @@ -3247,6 +3241,10 @@ private static boolean verifyArrayLength(Object array, int maxIndex, PacketStrea private static Object verifyString(long objectId, PacketStream reply, JDWPContext context) { Object string = context.getIds().fromId((int) objectId); + if (string == null) { + reply.errorCode(ErrorCodes.INVALID_OBJECT); + return null; + } if (!context.isString(string)) { reply.errorCode(ErrorCodes.INVALID_STRING);