Add proper synchronization for sketch launching and closing

Pressing Run button twice launched two sketch windows, but only one of
them could be closed by Stop button. Pressing Stop had effect only after
sketch VM was launched.

Both of these issues are now fixed and buttons can handle a
frustration-relieving session of aggressive clicking, leaving one sketch
window if Run was the last button pressed or no window if Stop was the
last button pressed.

java/src/processing/mode/java/JavaEditor.java

@@ -47,6 +47,9 @@ public class JavaEditor extends Editor {
   // Runner associated with this editor window
   private Runner runtime;
 
+  private boolean runtimeLaunchRequested;
+  private final Object runtimeLock = new Object[0];
+
   // Need to sort through the rest of these additions [fry]
 
   protected final List<LineHighlight> breakpointedLines = new ArrayList<>();
@@ -1057,57 +1060,48 @@ public class JavaEditor extends Editor {
       debugger.continueDebug();
 
     } else {
-      prepareRun();
-      toolbar.activateRun();
-      new Thread(new Runnable() {
-        public void run() {
-          try {
-            //runtime = jmode.handleRun(sketch, JavaEditor.this);
-            runtime = jmode.handleLaunch(sketch, JavaEditor.this, false);
-          } catch (Exception e) {
-            EventQueue.invokeLater(() -> statusError(e));
-          }
-        }
-      }).start();
+      handleLaunch(false, false);
     }
   }
 
 
   public void handlePresent() {
-    prepareRun();
-    toolbar.activateRun();
-    new Thread(new Runnable() {
-      public void run() {
-        try {
-          //runtime = jmode.handlePresent(sketch, JavaEditor.this);
-          runtime = jmode.handleLaunch(sketch, JavaEditor.this, true);
-        } catch (Exception e) {
-          EventQueue.invokeLater(() -> statusError(e));
-        }
-      }
-    }).start();
+    handleLaunch(true, false);
   }
 
 
   public void handleTweak() {
-    prepareRun();
-    //toolbar.activate(JavaToolbar.RUN);
-    toolbar.activateRun();
+    autoSave();
 
     if (sketch.isModified()) {
-      toolbar.deactivateRun();
       Messages.showMessage(Language.text("menu.file.save"),
                            Language.text("tweak_mode.save_before_tweak"));
       return;
     }
 
-    new Thread(new Runnable() {
-      public void run() {
-        try {
-          runtime = jmode.handleTweak(sketch, JavaEditor.this);
-        } catch (Exception e) {
-          EventQueue.invokeLater(() -> statusError(e));
+    handleLaunch(false, true);
+  }
+
+  protected void handleLaunch(boolean present, boolean tweak) {
+    prepareRun();
+    toolbar.activateRun();
+    synchronized (runtimeLock) {
+      runtimeLaunchRequested = true;
+    }
+    new Thread(() -> {
+      try {
+        synchronized (runtimeLock) {
+          if (runtimeLaunchRequested) {
+            runtimeLaunchRequested = false;
+            if (!tweak) {
+              runtime = jmode.handleLaunch(sketch, JavaEditor.this, present);
+            } else {
+              runtime = jmode.handleTweak(sketch, JavaEditor.this);
+            }
+          }
         }
+      } catch (Exception e) {
+        EventQueue.invokeLater(() -> statusError(e));
       }
     }).start();
   }
@@ -1122,23 +1116,24 @@ public class JavaEditor extends Editor {
       debugger.stopDebug();
 
     } else {
-//      toolbar.activate(JavaToolbar.STOP);
       toolbar.activateStop();
 
       try {
-        //jmode.handleStop();
-        if (runtime != null) {
-          runtime.close();  // kills the window
-          runtime = null;
-          //      } else {
-          //        System.out.println("runtime is null");
+        synchronized (runtimeLock) {
+          if (runtimeLaunchRequested) {
+            // Cancel the launch before the runtime was created
+            runtimeLaunchRequested = false;
+          }
+          if (runtime != null) {
+            // Cancel the launch after the runtime was created
+            runtime.close();  // kills the window
+            runtime = null;
+          }
         }
       } catch (Exception e) {
         statusError(e);
       }
 
-//      toolbar.deactivate(JavaToolbar.RUN);
-//      toolbar.deactivate(JavaToolbar.STOP);
       toolbar.deactivateStop();
       toolbar.deactivateRun();
 
@@ -1171,8 +1166,10 @@ public class JavaEditor extends Editor {
 
 
   public void onRunnerExiting(Runner runner) {
-    if (this.runtime == runner) {
-      deactivateRun();
+    synchronized (runtimeLock) {
+      if (this.runtime == runner) {
+        deactivateRun();
+      }
     }
   }
 

java/src/processing/mode/java/runner/Runner.java

@@ -61,7 +61,7 @@ public class Runner implements MessageConsumer {
   protected RunnerListener listener;
 
   // Running remote VM
-  protected VirtualMachine vm;
+  protected volatile VirtualMachine vm;
   protected boolean vmReturnedError;
 
   // Thread transferring remote error stream to our error stream
@@ -78,6 +78,9 @@ public class Runner implements MessageConsumer {
   protected PrintStream sketchErr;
   protected PrintStream sketchOut;
 
+  protected volatile boolean cancelled;
+  protected final Object cancelLock = new Object[0];
+
 
   public Runner(JavaBuild build, RunnerListener listener) throws SketchException {
     this.listener = listener;
@@ -218,6 +221,13 @@ public class Runner implements MessageConsumer {
 
     commandArgs.append(vmParams);
     commandArgs.append(sketchParams);
+
+    // Opportunistically quit if the launch was cancelled,
+    // the next chance to cancel will be after connecting to the VM
+    if (cancelled) {
+      return false;
+    }
+
     launchJava(commandArgs.array());
 
     AttachingConnector connector = (AttachingConnector)
@@ -249,7 +259,15 @@ public class Runner implements MessageConsumer {
       while (true) {
         try {
           Messages.log(getClass().getName() + " attempting to attach to VM");
-          vm = connector.attach(arguments);
+          synchronized (cancelLock) {
+            vm = connector.attach(arguments);
+            if (cancelled && vm != null) {
+              // cancelled and connected to the VM, handle closing now
+              Messages.log(getClass().getName() + " aborting, launch cancelled");
+              close();
+              return false;
+            }
+          }
 //          vm = connector.attach(arguments);
           if (vm != null) {
             Messages.log(getClass().getName() + " attached to the VM");
@@ -523,29 +541,28 @@ public class Runner implements MessageConsumer {
     //vm.setDebugTraceMode(debugTraceMode);
 //    vm.setDebugTraceMode(VirtualMachine.TRACE_ALL);
 //    vm.setDebugTraceMode(VirtualMachine.TRACE_NONE);  // formerly, seems to have no effect
+    try {
+      // Calling this seems to set something internally to make the
+      // Eclipse JDI wake up. Without it, an ObjectCollectedException
+      // is thrown on excReq.enable(). No idea why this works,
+      // but at least exception handling has returned. (Suspect that it may
+      // block until all or at least some threads are available, meaning
+      // that the app has launched and we have legit objects to talk to).
+      vm.allThreads();
+      // The bug may not have been noticed because the test suite waits for
+      // a thread to be available, and queries it by calling allThreads().
+      // See org.eclipse.debug.jdi.tests.AbstractJDITest for the example.
 
-    // Calling this seems to set something internally to make the
-    // Eclipse JDI wake up. Without it, an ObjectCollectedException
-    // is thrown on excReq.enable(). No idea why this works,
-    // but at least exception handling has returned. (Suspect that it may
-    // block until all or at least some threads are available, meaning
-    // that the app has launched and we have legit objects to talk to).
-    vm.allThreads();
-    // The bug may not have been noticed because the test suite waits for
-    // a thread to be available, and queries it by calling allThreads().
-    // See org.eclipse.debug.jdi.tests.AbstractJDITest for the example.
-
-    EventRequestManager mgr = vm.eventRequestManager();
-    // get only the uncaught exceptions
-    ExceptionRequest excReq = mgr.createExceptionRequest(null, false, true);
-//    System.out.println(excReq);
-    // this version reports all exceptions, caught or uncaught
-//  ExceptionRequest excReq = mgr.createExceptionRequest(null, true, true);
-    // suspend so we can step
-    excReq.setSuspendPolicy(EventRequest.SUSPEND_ALL);
-//    excReq.setSuspendPolicy(EventRequest.SUSPEND_EVENT_THREAD);
-//    excReq.setSuspendPolicy(EventRequest.SUSPEND_NONE);  // another option?
-    excReq.enable();
+      EventRequestManager mgr = vm.eventRequestManager();
+      // get only the uncaught exceptions
+      ExceptionRequest excReq = mgr.createExceptionRequest(null, false, true);
+      // this version reports all exceptions, caught or uncaught
+      // suspend so we can step
+      excReq.setSuspendPolicy(EventRequest.SUSPEND_ALL);
+      excReq.enable();
+    } catch (VMDisconnectedException ignore) {
+      return;
+    }
 
     Thread eventThread = new Thread() {
       public void run() {
@@ -851,19 +868,22 @@ public class Runner implements MessageConsumer {
 
 
   public void close() {
-    // TODO make sure stop() has already been called to exit the sketch
+    synchronized (cancelLock) {
+      cancelled = true;
 
-    // TODO actually kill off the vm here
-    if (vm != null) {
-      try {
-        vm.exit(0);
+      // TODO make sure stop() has already been called to exit the sketch
 
-      } catch (com.sun.jdi.VMDisconnectedException vmde) {
-        // if the vm has disconnected on its own, ignore message
-        //System.out.println("harmless disconnect " + vmde.getMessage());
-        // TODO shouldn't need to do this, need to do more cleanup
+      // TODO actually kill off the vm here
+      if (vm != null) {
+        try {
+          vm.exit(0);
+
+        } catch (com.sun.jdi.VMDisconnectedException vmde) {
+          // if the vm has disconnected on its own, ignore message
+          //System.out.println("harmless disconnect " + vmde.getMessage());
+          // TODO shouldn't need to do this, need to do more cleanup
+        }
       }
-      vm = null;
     }
   }
 
@@ -884,7 +904,9 @@ public class Runner implements MessageConsumer {
       if (editor != null) {
 //        editor.internalCloseRunner();  // [091124]
 //        editor.handleStop();  // prior to 0192
-        editor.internalCloseRunner();  // 0192
+        java.awt.EventQueue.invokeLater(() -> {
+          editor.internalCloseRunner();  // 0192
+        });
       }
       return;
     }