Attempt to reduce race condition on exit when connections are open.

nwflte · Aug 31, 2022 · 6fd5fc5 · 6fd5fc5
1 parent e340017
commit 6fd5fc5
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 53 deletions.
diff --git a/...racle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWPInstrument.java b/...racle.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<Thread> 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/...cle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/SocketConnection.java b/...cle.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<PacketStream> 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