Add proper synchronization for sketch launching and closing

JakubValtar · JakubValtar · commit aa2db6b505ff · 2017-01-23T03:07:42.000+01:00
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.
diff --git a/java/src/processing/mode/java/JavaEditor.java b/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 void handleRun() {
       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 void handleStop() {
       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 void handleContinue() {
 
 
   public void onRunnerExiting(Runner runner) {
-    if (this.runtime == runner) {
-      deactivateRun();
+    synchronized (runtimeLock) {
+      if (this.runtime == runner) {
+        deactivateRun();
+      }
     }
   }
 
diff --git a/java/src/processing/mode/java/runner/Runner.java b/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 boolean launchVirtualMachine(boolean present, String[] args) {
 
     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 boolean launchVirtualMachine(boolean present, String[] args) {
       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 @@ protected void generateTrace() {
     //vm.setDebugTraceMode(debugTraceMode);
 //    vm.setDebugTraceMode(VirtualMachine.TRACE_ALL);
 //    vm.setDebugTraceMode(VirtualMachine.TRACE_NONE);  // formerly, seems to have no effect
-
-    // 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();
+    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.
+
+      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 @@ protected SketchException findException(String message, ObjectReference or, Thre
 
 
   public void close() {
-    // TODO make sure stop() has already been called to exit the sketch
+    synchronized (cancelLock) {
+      cancelled = true;
+
+      // TODO make sure stop() has already been called to exit the sketch
 
-    // TODO actually kill off the vm here
-    if (vm != null) {
-      try {
-        vm.exit(0);
+      // 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
+        } 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 @@ synchronized public void message(String s) {
       if (editor != null) {
 //        editor.internalCloseRunner();  // [091124]
 //        editor.handleStop();  // prior to 0192
-        editor.internalCloseRunner();  // 0192
+        java.awt.EventQueue.invokeLater(() -> {
+          editor.internalCloseRunner();  // 0192
+        });
       }
       return;
     }
