From 0040c8a5f8e345346202cf7f6d20692af5d036f7 Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Fri, 13 May 2022 02:05:47 +0200 Subject: [PATCH 1/3] Added catching executors --- .../cryptomator/common/CatchingExecutors.java | 99 +++++++++++++++++++ .../org/cryptomator/common/CommonsModule.java | 9 +- 2 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 src/main/java/org/cryptomator/common/CatchingExecutors.java diff --git a/src/main/java/org/cryptomator/common/CatchingExecutors.java b/src/main/java/org/cryptomator/common/CatchingExecutors.java new file mode 100644 index 000000000..59bacbed7 --- /dev/null +++ b/src/main/java/org/cryptomator/common/CatchingExecutors.java @@ -0,0 +1,99 @@ +package org.cryptomator.common; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javafx.application.Platform; +import javafx.concurrent.Task; +import java.util.Objects; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + +//Inspired by: https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/ThreadPoolExecutor.html#afterExecute(java.lang.Runnable,java.lang.Throwable) +public final class CatchingExecutors { + + private static final Logger LOG = LoggerFactory.getLogger(CatchingExecutors.class); + + private CatchingExecutors() { /* NO-OP */ } + + public static class CatchingScheduledThreadPoolExecutor extends ScheduledThreadPoolExecutor { + + public CatchingScheduledThreadPoolExecutor(int corePoolSize, ThreadFactory threadFactory) { + super(corePoolSize, threadFactory); + } + + @Override + protected void afterExecute(Runnable runnable, Throwable throwable) { + super.afterExecute(runnable, throwable); + afterExecute0(runnable, throwable); + } + } + + public static class CatchingThreadPoolExecutor extends ThreadPoolExecutor { + + public CatchingThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit, BlockingQueue workQueue, ThreadFactory threadFactory) { + super(corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue, threadFactory); + } + + @Override + protected void afterExecute(Runnable runnable, Throwable throwable) { + super.afterExecute(runnable, throwable); + afterExecute0(runnable, throwable); + } + } + + private static void afterExecute0(Runnable runnable, Throwable throwable) { + if (throwable == null) { + if (runnable instanceof Task) { + afterExecuteTask(Thread.currentThread(), (Task) runnable); + return; + } + throwable = getThrowable(runnable); + } + + if (throwable != null) { + callHandler(Thread.currentThread(), throwable); + } + //Errors in this method are delegated to the UncaughtExceptionHandler of the current thread + } + + private static void callHandler(Thread thread, Throwable throwable) { + Objects.requireNonNullElseGet(thread.getUncaughtExceptionHandler(), CatchingExecutors::fallbackHandler).uncaughtException(thread, throwable); + } + + private static Thread.UncaughtExceptionHandler fallbackHandler() { + return (thread, throwable) -> LOG.error("FALLBACK: Uncaught exception in " + thread.getName(), throwable); + } + + private static void afterExecuteTask(Thread caller, Task task) { + Platform.runLater(() -> { + if (task.getOnFailed() == null) { + callHandler(caller, task.getException()); + } + }); + } + + private static Throwable getThrowable(Runnable runnable) { + assert !(runnable instanceof Task); + + if (runnable instanceof Future && ((Future) runnable).isDone()) { + try { + ((Future) runnable).get(); + } catch (CancellationException ce) { + return ce; + } catch (ExecutionException ee) { + return ee.getCause(); + } catch (InterruptedException ie) { + //Ignore/Reset + Thread.currentThread().interrupt(); + } + } + return null; + } +} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/common/CommonsModule.java b/src/main/java/org/cryptomator/common/CommonsModule.java index eb7e6cd13..edb4f063b 100644 --- a/src/main/java/org/cryptomator/common/CommonsModule.java +++ b/src/main/java/org/cryptomator/common/CommonsModule.java @@ -12,9 +12,7 @@ import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.keychain.KeychainModule; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.settings.SettingsProvider; -import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultComponent; -import org.cryptomator.common.vaults.VaultListManager; import org.cryptomator.common.vaults.VaultListModule; import org.cryptomator.cryptolib.common.MasterkeyFileAccess; import org.cryptomator.frontend.webdav.WebDavServer; @@ -25,16 +23,13 @@ import javax.inject.Named; import javax.inject.Singleton; import javafx.beans.binding.Binding; import javafx.beans.binding.Bindings; -import javafx.collections.ObservableList; import java.net.InetSocketAddress; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.util.Comparator; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.SynchronousQueue; -import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -93,7 +88,7 @@ public abstract class CommonsModule { @Singleton static ScheduledExecutorService provideScheduledExecutorService(ShutdownHook shutdownHook) { final AtomicInteger threadNumber = new AtomicInteger(1); - ScheduledExecutorService executorService = Executors.newScheduledThreadPool(NUM_SCHEDULER_THREADS, r -> { + ScheduledExecutorService executorService = new CatchingExecutors.CatchingScheduledThreadPoolExecutor(NUM_SCHEDULER_THREADS, r -> { String name = String.format("App Scheduled Executor %02d", threadNumber.getAndIncrement()); Thread t = new Thread(r); t.setName(name); @@ -110,7 +105,7 @@ public abstract class CommonsModule { @Singleton static ExecutorService provideExecutorService(ShutdownHook shutdownHook) { final AtomicInteger threadNumber = new AtomicInteger(1); - ExecutorService executorService = new ThreadPoolExecutor(NUM_CORE_BG_THREADS, Integer.MAX_VALUE, BG_THREAD_KEEPALIVE_SECONDS, TimeUnit.SECONDS, new SynchronousQueue<>(), r -> { + ExecutorService executorService = new CatchingExecutors.CatchingThreadPoolExecutor(NUM_CORE_BG_THREADS, Integer.MAX_VALUE, BG_THREAD_KEEPALIVE_SECONDS, TimeUnit.SECONDS, new SynchronousQueue<>(), r -> { String name = String.format("App Background Thread %03d", threadNumber.getAndIncrement()); Thread t = new Thread(r); t.setName(name); From e9a71827edb1afa90bd7fb3cc51dd80589220d8c Mon Sep 17 00:00:00 2001 From: JaniruTEC <52893617+JaniruTEC@users.noreply.github.com> Date: Fri, 27 May 2022 01:36:10 +0200 Subject: [PATCH 2/3] Apply suggestions from code review Establishing symmetry with `afterExecuteTask` Co-authored-by: Sebastian Stenzel --- .../cryptomator/common/CatchingExecutors.java | 38 ++++++++----------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/cryptomator/common/CatchingExecutors.java b/src/main/java/org/cryptomator/common/CatchingExecutors.java index 59bacbed7..469fc3741 100644 --- a/src/main/java/org/cryptomator/common/CatchingExecutors.java +++ b/src/main/java/org/cryptomator/common/CatchingExecutors.java @@ -49,16 +49,12 @@ public final class CatchingExecutors { } private static void afterExecute0(Runnable runnable, Throwable throwable) { - if (throwable == null) { - if (runnable instanceof Task) { - afterExecuteTask(Thread.currentThread(), (Task) runnable); - return; - } - throwable = getThrowable(runnable); - } - if (throwable != null) { callHandler(Thread.currentThread(), throwable); + } else if (runnable instanceof Task t) { + afterExecuteTask(Thread.currentThread(), t); + } else if (runnable instanceof Future f) { + afterExecuteFuture(f); } //Errors in this method are delegated to the UncaughtExceptionHandler of the current thread } @@ -79,21 +75,17 @@ public final class CatchingExecutors { }); } - private static Throwable getThrowable(Runnable runnable) { - assert !(runnable instanceof Task); - - if (runnable instanceof Future && ((Future) runnable).isDone()) { - try { - ((Future) runnable).get(); - } catch (CancellationException ce) { - return ce; - } catch (ExecutionException ee) { - return ee.getCause(); - } catch (InterruptedException ie) { - //Ignore/Reset - Thread.currentThread().interrupt(); - } + private static void afterExecuteFuture(Future future) { + assert future.isDone(); + try { + future.get(); + } catch (CancellationException ce) { + callHandler(Thread.currentThread(), ce); + } catch (ExecutionException ee) { + callHandler(Thread.currentThread(), ee.getCause()); + } catch (InterruptedException ie) { + //Ignore/Reset + Thread.currentThread().interrupt(); } - return null; } } \ No newline at end of file From f740a93b6c0386ad8a3f4b3cdf2a10efd331a767 Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Fri, 27 May 2022 01:23:06 +0200 Subject: [PATCH 3/3] Applied suggestions from code review Changed signature of afterExecuteTask See: https://github.com/cryptomator/cryptomator/pull/2259/files#r875548549 -- Changed name of "afterExecute0" to "afterExecuteInternal" See: https://github.com/cryptomator/cryptomator/pull/2259/files#r875551994 --- .../org/cryptomator/common/CatchingExecutors.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/cryptomator/common/CatchingExecutors.java b/src/main/java/org/cryptomator/common/CatchingExecutors.java index 469fc3741..900f81ec9 100644 --- a/src/main/java/org/cryptomator/common/CatchingExecutors.java +++ b/src/main/java/org/cryptomator/common/CatchingExecutors.java @@ -31,7 +31,7 @@ public final class CatchingExecutors { @Override protected void afterExecute(Runnable runnable, Throwable throwable) { super.afterExecute(runnable, throwable); - afterExecute0(runnable, throwable); + afterExecuteInternal(runnable, throwable); } } @@ -44,15 +44,15 @@ public final class CatchingExecutors { @Override protected void afterExecute(Runnable runnable, Throwable throwable) { super.afterExecute(runnable, throwable); - afterExecute0(runnable, throwable); + afterExecuteInternal(runnable, throwable); } } - private static void afterExecute0(Runnable runnable, Throwable throwable) { + private static void afterExecuteInternal(Runnable runnable, Throwable throwable) { if (throwable != null) { callHandler(Thread.currentThread(), throwable); } else if (runnable instanceof Task t) { - afterExecuteTask(Thread.currentThread(), t); + afterExecuteTask(t); } else if (runnable instanceof Future f) { afterExecuteFuture(f); } @@ -67,7 +67,8 @@ public final class CatchingExecutors { return (thread, throwable) -> LOG.error("FALLBACK: Uncaught exception in " + thread.getName(), throwable); } - private static void afterExecuteTask(Thread caller, Task task) { + private static void afterExecuteTask(Task task) { + var caller = Thread.currentThread(); Platform.runLater(() -> { if (task.getOnFailed() == null) { callHandler(caller, task.getException());