From 6fd5fc5acde8b659bbdad2ece7070c00d8364f47 Mon Sep 17 00:00:00 2001 From: Danilo Ansaloni Date: Tue, 30 Aug 2022 18:36:39 +0200 Subject: [PATCH] Attempt to reduce race condition on exit when connections are open. --- .../espresso/jdwp/impl/JDWPInstrument.java | 72 +++++++++---------- .../espresso/jdwp/impl/SocketConnection.java | 27 +++---- 2 files changed, 46 insertions(+), 53 deletions(-) diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWPInstrument.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWPInstrument.java index 85bcb4226085..98a5a8c2b12b 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWPInstrument.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWPInstrument.java @@ -28,6 +28,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.concurrent.Callable; +import java.util.concurrent.Semaphore; import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.exception.AbstractTruffleException; @@ -50,7 +51,7 @@ public final class JDWPInstrument extends TruffleInstrument implements Runnable private Collection activeThreads = new ArrayList<>(); private PrintStream err; - private volatile boolean resetting; + private final Semaphore resetting = new Semaphore(1); @Override protected void onCreate(TruffleInstrument.Env instrumentEnv) { @@ -63,53 +64,52 @@ protected void onCreate(TruffleInstrument.Env instrumentEnv) { } public void reset(boolean prepareForReconnect) { + if (!resetting.tryAcquire()) { + return; + } // stop all running jdwp threads in an orderly fashion - resetting = true; - try { + for (Thread activeThread : activeThreads) { + activeThread.interrupt(); + } + // close the connection to the debugger + if (connection != null) { + connection.close(); + } + // wait for threads to fully stop + boolean stillRunning = true; + while (stillRunning) { + stillRunning = false; for (Thread activeThread : activeThreads) { - activeThread.interrupt(); - } - // close the connection to the debugger - if (connection != null) { - connection.close(); - } - // wait for threads to fully stop - boolean stillRunning = true; - while (stillRunning) { - stillRunning = false; - for (Thread activeThread : activeThreads) { - if (activeThread.isAlive()) { - stillRunning = true; - } - } - try { - Thread.sleep(10); - } catch (InterruptedException e) { - // ignore + if (activeThread.isAlive()) { + stillRunning = true; } } + try { + Thread.sleep(10); + } catch (InterruptedException e) { + // ignore + } + } - // re-enable GC for all objects - controller.getGCPrevention().clearAll(); + // re-enable GC for all objects + controller.getGCPrevention().clearAll(); - // end the current debugger session to avoid hitting any further breakpoints - // when resuming all threads - controller.endSession(); + // end the current debugger session to avoid hitting any further breakpoints + // when resuming all threads + controller.endSession(); - // resume all threads - controller.resumeAll(true); + // resume all threads + controller.resumeAll(true); - if (prepareForReconnect) { - // replace the controller instance - controller.reInitialize(); - } - } finally { - resetting = false; + if (prepareForReconnect) { + // replace the controller instance + controller.reInitialize(); + resetting.release(); } } public boolean isResetting() { - return resetting; + return resetting.availablePermits() == 0; } public void printStackTrace(Throwable e) { diff --git a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SocketConnection.java b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SocketConnection.java index 954f38ccf364..13b832ee37f9 100644 --- a/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SocketConnection.java +++ b/espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SocketConnection.java @@ -29,6 +29,7 @@ import java.net.Socket; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Semaphore; public final class SocketConnection implements Runnable { private final Socket socket; @@ -37,7 +38,7 @@ public final class SocketConnection implements Runnable { private final InputStream socketInput; private final Object receiveLock = new Object(); private final Object sendLock = new Object(); - private final Object closeLock = new Object(); + private final Semaphore isOpen = new Semaphore(1); private final BlockingQueue queue = new ArrayBlockingQueue<>(4096); @@ -47,7 +48,6 @@ public final class SocketConnection implements Runnable { socketInput = socket.getInputStream(); socketOutput = socket.getOutputStream(); } - public void close() throws IOException { // send outstanding packets before closing while (!queue.isEmpty()) { @@ -61,25 +61,18 @@ public void close() throws IOException { } } socketOutput.flush(); - - synchronized (closeLock) { - if (closed) { - return; - } - JDWP.LOGGER.fine("closing socket now"); - - socketOutput.close(); - socketInput.close(); - socket.close(); - queue.clear(); - closed = true; + if (!isOpen.tryAcquire()) { + return; } + JDWP.LOGGER.fine("closing socket now"); + socketOutput.close(); + socketInput.close(); + socket.close(); + queue.clear(); } public boolean isOpen() { - synchronized (closeLock) { - return !closed; - } + return isOpen.availablePermits() > 0; } @Override