From e1f44fb48ab3b6124cc892959cb735eea595e677 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 13 Feb 2020 16:29:49 +0100 Subject: [PATCH] App lifecycle fixes --- .../org/cryptomator/launcher/Cryptomator.java | 2 - .../launcher/FileOpenRequestHandler.java | 14 +++-- .../cryptomator/launcher/IpcProtocolImpl.java | 3 +- .../launcher/FileOpenRequestHandlerTest.java | 20 +++---- .../ui/launcher/AppLaunchEvent.java | 7 ++- .../ui/launcher/AppLifecycleListener.java | 57 +++++++++++-------- 6 files changed, 55 insertions(+), 48 deletions(-) diff --git a/main/launcher/src/main/java/org/cryptomator/launcher/Cryptomator.java b/main/launcher/src/main/java/org/cryptomator/launcher/Cryptomator.java index 763720d98..04eb9448d 100644 --- a/main/launcher/src/main/java/org/cryptomator/launcher/Cryptomator.java +++ b/main/launcher/src/main/java/org/cryptomator/launcher/Cryptomator.java @@ -5,7 +5,6 @@ *******************************************************************************/ package org.cryptomator.launcher; -import javafx.application.Platform; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.logging.DebugMode; import org.cryptomator.logging.LoggerConfiguration; @@ -91,7 +90,6 @@ public class Cryptomator { try { uiLauncher.launch(); shutdownLatch.await(); - Platform.exit(); LOG.info("UI shut down"); return 0; } catch (InterruptedException e) { diff --git a/main/launcher/src/main/java/org/cryptomator/launcher/FileOpenRequestHandler.java b/main/launcher/src/main/java/org/cryptomator/launcher/FileOpenRequestHandler.java index b0ea9d08b..cabcaf087 100644 --- a/main/launcher/src/main/java/org/cryptomator/launcher/FileOpenRequestHandler.java +++ b/main/launcher/src/main/java/org/cryptomator/launcher/FileOpenRequestHandler.java @@ -21,8 +21,10 @@ import java.nio.file.FileSystems; import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.util.Arrays; +import java.util.Collection; import java.util.Objects; import java.util.concurrent.BlockingQueue; +import java.util.stream.Collectors; import java.util.stream.Stream; @Singleton @@ -40,7 +42,7 @@ class FileOpenRequestHandler { } private void openFiles(OpenFilesEvent evt) { - Stream pathsToOpen = evt.getFiles().stream().map(File::toPath); + Collection pathsToOpen = evt.getFiles().stream().map(File::toPath).collect(Collectors.toList()); AppLaunchEvent launchEvent = new AppLaunchEvent(AppLaunchEvent.EventType.OPEN_FILE, pathsToOpen); tryToEnqueueFileOpenRequest(launchEvent); } @@ -51,16 +53,18 @@ class FileOpenRequestHandler { // visible for testing void handleLaunchArgs(FileSystem fs, String[] args) { - Stream pathsToOpen = Arrays.stream(args).map(str -> { + Collection pathsToOpen = Arrays.stream(args).map(str -> { try { return fs.getPath(str); } catch (InvalidPathException e) { LOG.trace("Argument not a valid path: {}", str); return null; } - }).filter(Objects::nonNull); - AppLaunchEvent launchEvent = new AppLaunchEvent(AppLaunchEvent.EventType.OPEN_FILE, pathsToOpen); - tryToEnqueueFileOpenRequest(launchEvent); + }).filter(Objects::nonNull).collect(Collectors.toList()); + if (!pathsToOpen.isEmpty()) { + AppLaunchEvent launchEvent = new AppLaunchEvent(AppLaunchEvent.EventType.OPEN_FILE, pathsToOpen); + tryToEnqueueFileOpenRequest(launchEvent); + } } diff --git a/main/launcher/src/main/java/org/cryptomator/launcher/IpcProtocolImpl.java b/main/launcher/src/main/java/org/cryptomator/launcher/IpcProtocolImpl.java index 4f6e8cacb..57f21f5e8 100644 --- a/main/launcher/src/main/java/org/cryptomator/launcher/IpcProtocolImpl.java +++ b/main/launcher/src/main/java/org/cryptomator/launcher/IpcProtocolImpl.java @@ -8,6 +8,7 @@ import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; import java.util.Arrays; +import java.util.Collections; import java.util.concurrent.BlockingQueue; import java.util.stream.Stream; @@ -27,7 +28,7 @@ class IpcProtocolImpl implements IpcProtocol { @Override public void revealRunningApp() { - launchEventQueue.add(new AppLaunchEvent(AppLaunchEvent.EventType.REVEAL_APP, Stream.empty())); + launchEventQueue.add(new AppLaunchEvent(AppLaunchEvent.EventType.REVEAL_APP, Collections.emptyList())); } @Override diff --git a/main/launcher/src/test/java/org/cryptomator/launcher/FileOpenRequestHandlerTest.java b/main/launcher/src/test/java/org/cryptomator/launcher/FileOpenRequestHandlerTest.java index b89f3ed82..b0ed4b307 100644 --- a/main/launcher/src/test/java/org/cryptomator/launcher/FileOpenRequestHandlerTest.java +++ b/main/launcher/src/test/java/org/cryptomator/launcher/FileOpenRequestHandlerTest.java @@ -15,16 +15,14 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.IOException; import java.nio.file.FileSystem; import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.List; +import java.util.Collection; +import java.util.Collections; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; -import java.util.stream.Collectors; -import java.util.stream.Stream; public class FileOpenRequestHandlerTest { @@ -39,32 +37,30 @@ public class FileOpenRequestHandlerTest { @Test @DisplayName("./cryptomator.exe foo bar") - public void testOpenArgsWithCorrectPaths() throws IOException { + public void testOpenArgsWithCorrectPaths() { inTest.handleLaunchArgs(new String[]{"foo", "bar"}); AppLaunchEvent evt = queue.poll(); Assertions.assertNotNull(evt); - List paths = evt.getPathsToOpen().collect(Collectors.toList()); + Collection paths = evt.getPathsToOpen(); MatcherAssert.assertThat(paths, CoreMatchers.hasItems(Paths.get("foo"), Paths.get("bar"))); } @Test @DisplayName("./cryptomator.exe foo (with 'foo' being an invalid path)") - public void testOpenArgsWithIncorrectPaths() throws IOException { + public void testOpenArgsWithIncorrectPaths() { FileSystem fs = Mockito.mock(FileSystem.class); Mockito.when(fs.getPath("foo")).thenThrow(new InvalidPathException("foo", "foo is not a path")); inTest.handleLaunchArgs(fs, new String[]{"foo"}); AppLaunchEvent evt = queue.poll(); - Assertions.assertNotNull(evt); - List paths = evt.getPathsToOpen().collect(Collectors.toList()); - Assertions.assertTrue(paths.isEmpty()); + Assertions.assertNull(evt); } @Test @DisplayName("./cryptomator.exe foo (with full event queue)") - public void testOpenArgsWithFullQueue() throws IOException { - queue.add(new AppLaunchEvent(AppLaunchEvent.EventType.OPEN_FILE, Stream.empty())); + public void testOpenArgsWithFullQueue() { + queue.add(new AppLaunchEvent(AppLaunchEvent.EventType.OPEN_FILE, Collections.emptyList())); Assumptions.assumeTrue(queue.remainingCapacity() == 0); inTest.handleLaunchArgs(new String[]{"foo"}); diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLaunchEvent.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLaunchEvent.java index 7fc7fcfe6..a7334302c 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLaunchEvent.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLaunchEvent.java @@ -1,16 +1,17 @@ package org.cryptomator.ui.launcher; import java.nio.file.Path; +import java.util.Collection; import java.util.stream.Stream; public class AppLaunchEvent { - private final Stream pathsToOpen; private final EventType type; + private final Collection pathsToOpen; public enum EventType {REVEAL_APP, OPEN_FILE} - public AppLaunchEvent(EventType type, Stream pathsToOpen) { + public AppLaunchEvent(EventType type, Collection pathsToOpen) { this.type = type; this.pathsToOpen = pathsToOpen; } @@ -19,7 +20,7 @@ public class AppLaunchEvent { return type; } - public Stream getPathsToOpen() { + public Collection getPathsToOpen() { return pathsToOpen; } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java index e06a23a63..bb53e8768 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java @@ -15,6 +15,7 @@ import javax.inject.Inject; import javax.inject.Named; import javax.inject.Singleton; import java.awt.Desktop; +import java.awt.EventQueue; import java.awt.desktop.QuitResponse; import java.util.EnumSet; import java.util.EventObject; @@ -31,14 +32,14 @@ public class AppLifecycleListener { private final FxApplicationStarter fxApplicationStarter; private final CountDownLatch shutdownLatch; private final ObservableList vaults; - private final AtomicBoolean allowSuddenTermination; + private final AtomicBoolean allowQuitWithoutPrompt; @Inject AppLifecycleListener(FxApplicationStarter fxApplicationStarter, @Named("shutdownLatch") CountDownLatch shutdownLatch, ShutdownHook shutdownHook, ObservableList vaults) { this.fxApplicationStarter = fxApplicationStarter; this.shutdownLatch = shutdownLatch; this.vaults = vaults; - this.allowSuddenTermination = new AtomicBoolean(true); + this.allowQuitWithoutPrompt = new AtomicBoolean(true); vaults.addListener(this::vaultListChanged); // register preferences shortcut @@ -51,11 +52,6 @@ public class AppLifecycleListener { Desktop.getDesktop().setQuitHandler(this::handleQuitRequest); } - // allow sudden termination - if (Desktop.getDesktop().isSupported(Desktop.Action.APP_SUDDEN_TERMINATION)) { - Desktop.getDesktop().enableSuddenTermination(); - } - shutdownHook.runOnShutdown(this::forceUnmountRemainingVaults); } @@ -66,7 +62,7 @@ public class AppLifecycleListener { handleQuitRequest(null, new QuitResponse() { @Override public void performQuit() { - shutdownLatch.countDown(); + System.exit(0); } @Override @@ -76,18 +72,37 @@ public class AppLifecycleListener { }); } + private void handleQuitRequest(@SuppressWarnings("unused") EventObject e, QuitResponse response) { + QuitResponse decoratedQuitResponse = decorateQuitResponse(response); + if (allowQuitWithoutPrompt.get()) { + decoratedQuitResponse.performQuit(); + } else { + fxApplicationStarter.get(true).thenAccept(app -> app.showQuitWindow(decoratedQuitResponse)); + } + } + + private QuitResponse decorateQuitResponse(QuitResponse originalQuitResponse) { + return new QuitResponse() { + @Override + public void performQuit() { + Platform.exit(); // will be no-op, if JavaFX never started. + shutdownLatch.countDown(); // main thread is waiting for this latch + EventQueue.invokeLater(originalQuitResponse::performQuit); // this will eventually call System.exit(0) + } + + @Override + public void cancelQuit() { + originalQuitResponse.cancelQuit(); + } + }; + } + private void vaultListChanged(@SuppressWarnings("unused") Observable observable) { assert Platform.isFxApplicationThread(); boolean allVaultsAllowTermination = vaults.stream().map(Vault::getState).allMatch(STATES_ALLOWING_TERMINATION::contains); - boolean suddenTerminationChanged = allowSuddenTermination.compareAndSet(!allVaultsAllowTermination, allVaultsAllowTermination); - if (suddenTerminationChanged && Desktop.getDesktop().isSupported(Desktop.Action.APP_SUDDEN_TERMINATION)) { - if (allVaultsAllowTermination) { - Desktop.getDesktop().enableSuddenTermination(); - LOG.debug("sudden termination enabled"); - } else { - Desktop.getDesktop().disableSuddenTermination(); - LOG.debug("sudden termination disabled"); - } + boolean suddenTerminationChanged = allowQuitWithoutPrompt.compareAndSet(!allVaultsAllowTermination, allVaultsAllowTermination); + if (suddenTerminationChanged) { + LOG.debug("Allow quitting without prompt: {}", allVaultsAllowTermination); } } @@ -95,14 +110,6 @@ public class AppLifecycleListener { fxApplicationStarter.get(true).thenAccept(app -> app.showPreferencesWindow(SelectedPreferencesTab.ANY)); } - private void handleQuitRequest(@SuppressWarnings("unused") EventObject e, QuitResponse response) { - if (allowSuddenTermination.get()) { - response.performQuit(); // really? - } else { - fxApplicationStarter.get(true).thenAccept(app -> app.showQuitWindow(response)); - } - } - private void forceUnmountRemainingVaults() { for (Vault vault : vaults) { if (vault.isUnlocked()) {