diff --git a/core/src/main/java/edu/wpi/grip/core/GRIPCoreModule.java b/core/src/main/java/edu/wpi/grip/core/GRIPCoreModule.java index 36b174884d..13f20d02e5 100644 --- a/core/src/main/java/edu/wpi/grip/core/GRIPCoreModule.java +++ b/core/src/main/java/edu/wpi/grip/core/GRIPCoreModule.java @@ -16,11 +16,15 @@ import edu.wpi.grip.core.sources.MultiImageFileSource; import edu.wpi.grip.core.util.ExceptionWitness; +import java.util.logging.Level; +import java.util.logging.Logger; + /** * A Guice {@link com.google.inject.Module} for GRIP's core package. This is where instances of {@link Pipeline}, * {@link Palette}, {@link Project}, etc... are created. */ public class GRIPCoreModule extends AbstractModule { + private final Logger logger = Logger.getLogger(GRIPCoreModule.class.getName()); private final EventBus eventBus = new EventBus(this::onSubscriberException); @@ -60,10 +64,20 @@ public void hear(TypeLiteral type, TypeEncounter encounter) { } private void onSubscriberException(Throwable exception, SubscriberExceptionContext context) { - eventBus.post(new UnexpectedThrowableEvent(exception, "An event subscriber threw an exception")); + if (exception instanceof InterruptedException) { + logger.log(Level.FINE, "EventBus Subscriber threw InterruptedException", exception); + Thread.currentThread().interrupt(); + } else { + eventBus.post(new UnexpectedThrowableEvent(exception, "An event subscriber threw an exception")); + } } private void onThreadException(Thread thread, Throwable exception) { - eventBus.post(new UnexpectedThrowableEvent(exception, thread + " threw an exception")); + if (exception instanceof InterruptedException) { + logger.log(Level.FINE, "InterruptedException from thread " + thread, exception); + Thread.currentThread().interrupt(); + } else { + eventBus.post(new UnexpectedThrowableEvent(exception, thread + " threw an exception")); + } } } diff --git a/core/src/main/java/edu/wpi/grip/core/StartStoppable.java b/core/src/main/java/edu/wpi/grip/core/StartStoppable.java index e51c67a5d7..a99630d932 100644 --- a/core/src/main/java/edu/wpi/grip/core/StartStoppable.java +++ b/core/src/main/java/edu/wpi/grip/core/StartStoppable.java @@ -26,7 +26,7 @@ public interface StartStoppable { * @throws TimeoutException If the thread fails to stop in a timely manner * @throws IOException If cleaning up some system resource fails. */ - void stop() throws TimeoutException, IOException; + void stop() throws InterruptedException, TimeoutException, IOException; /** * Used to indicate if the source is running or stopped diff --git a/core/src/main/java/edu/wpi/grip/core/Step.java b/core/src/main/java/edu/wpi/grip/core/Step.java index c6b58f5a8c..0843f40d2a 100644 --- a/core/src/main/java/edu/wpi/grip/core/Step.java +++ b/core/src/main/java/edu/wpi/grip/core/Step.java @@ -143,7 +143,9 @@ private synchronized void runPerformIfPossible() { try { this.operation.perform(inputSockets, outputSockets, data); - } catch (Exception e) { + } catch (RuntimeException e) { + // We do not want to catch all exceptions, only runtime exceptions. + // This is especially important when it comes to InterruptedExceptions final String operationFailedMessage = "The " + operation.getName() + " operation did not perform correctly."; logger.log(Level.WARNING, operationFailedMessage, e); witness.flagException(e, operationFailedMessage); diff --git a/core/src/main/java/edu/wpi/grip/core/sources/CameraSource.java b/core/src/main/java/edu/wpi/grip/core/sources/CameraSource.java index 7d3fc77739..7413da1be8 100644 --- a/core/src/main/java/edu/wpi/grip/core/sources/CameraSource.java +++ b/core/src/main/java/edu/wpi/grip/core/sources/CameraSource.java @@ -208,6 +208,8 @@ public void start() throws IOException, IllegalStateException { } catch (TimeoutException | IOException e) { // TODO: This should use the ExceptionWitness once that has a UI component added for it eventBus.post(new UnexpectedThrowableEvent(e, "Camera Frame Grabber could not be stopped!")); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); } } ); @@ -228,13 +230,13 @@ public void start() throws IOException, IllegalStateException { * @throws IOException If there is a problem stopping the Source * @throws IllegalStateException If the camera is already stopped. */ - public void stop() throws TimeoutException, IOException { + public void stop() throws InterruptedException, TimeoutException, IOException { synchronized (this) { if (frameThread.isPresent()) { final Thread ex = frameThread.get(); ex.interrupt(); try { - ex.join(TimeUnit.SECONDS.toMillis(500)); + ex.join(TimeUnit.SECONDS.toMillis(10)); if (ex.isAlive()) { throw new TimeoutException("Unable to terminate video feed from Web Camera"); } @@ -243,6 +245,7 @@ public void stop() throws TimeoutException, IOException { } catch (InterruptedException e) { Thread.currentThread().interrupt(); logger.log(Level.WARNING, e.getMessage(), e); + throw e; } finally { // This will always run even if a timeout exception occurs try { @@ -266,7 +269,7 @@ public synchronized boolean isStarted() { } @Subscribe - public void onSourceRemovedEvent(SourceRemovedEvent event) throws TimeoutException, IOException { + public void onSourceRemovedEvent(SourceRemovedEvent event) throws InterruptedException, TimeoutException, IOException { if (event.getSource() == this) { try { if (this.isStarted()) this.stop(); diff --git a/core/src/main/java/edu/wpi/grip/core/util/ExceptionWitness.java b/core/src/main/java/edu/wpi/grip/core/util/ExceptionWitness.java index e6d0c311e5..4c4c13554d 100644 --- a/core/src/main/java/edu/wpi/grip/core/util/ExceptionWitness.java +++ b/core/src/main/java/edu/wpi/grip/core/util/ExceptionWitness.java @@ -48,10 +48,16 @@ public interface Factory { /** * Indicates to the witness that an exception has occurred. This will also post an {@link ExceptionEvent} to the {@link EventBus} * - * @param exception The exception that this is reporting + * @param exception The exception that this is reporting. + * If the Exception is an InterruptedException then this will not post an exception, instead, + * it will set the threads interrupted state and return. * @param message Any additional details that should be associated with this message. */ public final void flagException(final Exception exception, final String message) { + if (exception instanceof InterruptedException) { + Thread.currentThread().interrupt(); + return; + } isExceptionState.set(true); this.eventBus.post(new ExceptionEvent(origin, exception, message)); } diff --git a/ui/src/main/java/edu/wpi/grip/ui/Main.java b/ui/src/main/java/edu/wpi/grip/ui/Main.java index f7369925c0..2832acacd5 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/Main.java +++ b/ui/src/main/java/edu/wpi/grip/ui/Main.java @@ -92,28 +92,35 @@ public void start(Stage stage) throws Exception { public final void onUnexpectedThrowableEvent(UnexpectedThrowableEvent event) { // Print throwable before showing the exception so that errors are in order in the console event.getThrowable().printStackTrace(); - // This should still use PlatformImpl - PlatformImpl.runAndWait(() -> { - // WARNING! Do not post any events from within this! It could result in a deadlock! - synchronized (this.dialogLock) { - try { - // Don't create more than one exception dialog at the same time - final ExceptionAlert exceptionAlert = new ExceptionAlert(root, event.getThrowable(), event.getMessage(), event.isFatal(), getHostServices()); - exceptionAlert.setInitialFocus(); - exceptionAlert.showAndWait(); - } catch (Throwable e) { - // Well in this case something has gone very, very wrong - // We don't want to create a feedback loop either. - e.printStackTrace(); - System.exit(1); // Ensure we shut down the application if we get an exception - } - } - }); - if (event.isFatal()) { - System.err.println("Original fatal exception"); - event.getThrowable().printStackTrace(); - System.exit(1); + try { + // If this is an interrupted exception then don't show an error. + // We should exit as fast as possible. + if (!(event.getThrowable() instanceof InterruptedException)) { + // This should still use PlatformImpl + PlatformImpl.runAndWait(() -> { + // WARNING! Do not post any events from within this! It could result in a deadlock! + synchronized (this.dialogLock) { + try { + // Don't create more than one exception dialog at the same time + final ExceptionAlert exceptionAlert = new ExceptionAlert(root, event.getThrowable(), event.getMessage(), event.isFatal(), getHostServices()); + exceptionAlert.setInitialFocus(); + exceptionAlert.showAndWait(); + } catch (Throwable e) { + // Well in this case something has gone very, very wrong + // We don't want to create a feedback loop either. + e.printStackTrace(); + System.exit(1); // Ensure we shut down the application if we get an exception + } + } + }); + } + } finally { + if (event.isFatal()) { + System.err.println("Original fatal exception"); + event.getThrowable().printStackTrace(); + System.exit(1); + } } } } diff --git a/ui/src/main/java/edu/wpi/grip/ui/components/StartStoppableButton.java b/ui/src/main/java/edu/wpi/grip/ui/components/StartStoppableButton.java index 3a0d99de91..88d7668092 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/components/StartStoppableButton.java +++ b/ui/src/main/java/edu/wpi/grip/ui/components/StartStoppableButton.java @@ -64,6 +64,9 @@ public interface Factory { // If this fails then an StartedStoppedEvent will not be posted } catch (TimeoutException | IOException e) { eventBus.post(new UnexpectedThrowableEvent(e, "Failed to stop")); + } catch (InterruptedException e) { + // Unfortunately we have to eat the exception here because we can't rethrow it + Thread.currentThread().interrupt(); } }); diff --git a/ui/src/main/java/edu/wpi/grip/ui/util/GRIPPlatform.java b/ui/src/main/java/edu/wpi/grip/ui/util/GRIPPlatform.java index 4e6bbbf28c..a661320bbb 100644 --- a/ui/src/main/java/edu/wpi/grip/ui/util/GRIPPlatform.java +++ b/ui/src/main/java/edu/wpi/grip/ui/util/GRIPPlatform.java @@ -49,6 +49,7 @@ public Runnable getAction() { * JavaFX application thread. If we are already on the JavaFX application thread * then this will be run immediately. Otherwise, it will be run as an event * inside of the event bus. + * If * If {@link #runAsSoonAsPossible(Runnable)} is called within itself it will always run * immediately because the runnable will always be run in the JavaFX thread. * @@ -69,25 +70,28 @@ public void runAsSoonAsPossible(Runnable action) { } @Subscribe - public void onJavaFXRunnerEvent(JavaFXRunnerEvent event) { + public void onJavaFXRunnerEvent(JavaFXRunnerEvent event) throws InterruptedException { assert !Platform.isFxApplicationThread() : "This should never be run on the application thread. This can cause a deadlock!"; + final Thread callingThread = Thread.currentThread(); + if (Thread.interrupted()) { + throw new InterruptedException("Interrupted in onJavaFXRunnerEvent"); + } // queue on JavaFX thread and wait for completion final CountDownLatch doneLatch = new CountDownLatch(1); Platform.runLater(() -> { try { - event.getAction().run(); + // If the calling thread was interrupted then don't run this event. + if(!callingThread.isInterrupted()) { + event.getAction().run(); + } } finally { doneLatch.countDown(); } }); - try { - while (!doneLatch.await(500, TimeUnit.MILLISECONDS)) { - logger.log(Level.WARNING, "POTENTIAL DEADLOCK!"); - } - } catch (InterruptedException e) { - // ignore exception + while (!doneLatch.await(500, TimeUnit.MILLISECONDS)) { + logger.log(Level.WARNING, "POTENTIAL DEADLOCK!"); } } diff --git a/ui/src/test/java/edu/wpi/grip/ui/util/GRIPPlatformTest.java b/ui/src/test/java/edu/wpi/grip/ui/util/GRIPPlatformTest.java index cc55439861..ba9d99f0cb 100644 --- a/ui/src/test/java/edu/wpi/grip/ui/util/GRIPPlatformTest.java +++ b/ui/src/test/java/edu/wpi/grip/ui/util/GRIPPlatformTest.java @@ -14,6 +14,7 @@ import java.util.logging.Logger; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class GRIPPlatformTest extends ApplicationTest { @@ -76,4 +77,15 @@ public void testRunAsSoonAsPossibleDoesNotDeadlockWhenRunInsideItself() throws E waiter.await(); } + @Test + public void testRunAsSoonAsPossibleWillNotCallIfInterrupted() throws Exception { + final boolean hasRun [] = {false}; + + Thread.currentThread().interrupt(); + platform.runAsSoonAsPossible(() -> { + hasRun[0] = true; + }); + assertFalse("runAsSoonAsPossible ran when interrupted", hasRun[0]); + } + } \ No newline at end of file