Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions core/src/main/java/edu/wpi/grip/core/GRIPCoreModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -60,10 +64,20 @@ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> 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"));
}
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/edu/wpi/grip/core/StartStoppable.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/java/edu/wpi/grip/core/Step.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
);
Expand All @@ -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");
}
Expand All @@ -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 {
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
49 changes: 28 additions & 21 deletions ui/src/main/java/edu/wpi/grip/ui/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
});

Expand Down
20 changes: 12 additions & 8 deletions ui/src/main/java/edu/wpi/grip/ui/util/GRIPPlatform.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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!");
}
}

Expand Down
12 changes: 12 additions & 0 deletions ui/src/test/java/edu/wpi/grip/ui/util/GRIPPlatformTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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]);
}

}