fix: Use ScheduledExecutorService with uncaughtExceptionHandler

Author: vorburger

src/dev/enola/be/task/TaskExecutor.java

@@ -3,6 +3,8 @@
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.MINUTES;
 
+import dev.enola.common.concurrent.Executors;
+
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Map;
@@ -11,7 +13,6 @@
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.FutureTask;
 import java.util.concurrent.ScheduledExecutorService;
@@ -29,15 +30,20 @@ public class TaskExecutor implements AutoCloseable {
     // can still find them later, with a separate eviction policy for that persistent store.
     private final Map<UUID, Task<?, ?>> tasks = new ConcurrentHashMap<>();
 
-    private final ExecutorService executor = TaskExecutorServices.newVirtualThreadPerTaskExecutor();
-
-    // TODO Use LoggingScheduledExecutorService from Enola Commons, for both.
+    // Nota bene: In *THEORY* we should *NEVER* have *ANY* uncaught exceptions from Task,
+    // because any exception thrown by the task's `execute()` method would be caught and
+    // wrapped in an ExecutionException by the Future returned by ExecutorService.submit().
+    //
+    // But in practice, who knows what the future holds, so we better log them just in case;
+    // just because "swallowed" lost exceptions are seriously the worst kind of bugs to diagnose!
+    private final ExecutorService executor =
+            Executors.newVirtualThreadPerTaskExecutorWithLoggingThreadUncaughtExceptionHandler(LOG);
 
     private final ScheduledExecutorService timeoutScheduler =
-            Executors.newSingleThreadScheduledExecutor();
+            Executors.newSingleThreadScheduledExecutor("TaskExecutor-Timeout", LOG);
 
     private final ScheduledExecutorService cleanupScheduler =
-            Executors.newSingleThreadScheduledExecutor();
+            Executors.newSingleThreadScheduledExecutor("TaskExecutor-Cleanup", LOG);
 
     public TaskExecutor(Duration completedTaskEvictionInterval) {
         var m = completedTaskEvictionInterval.toMinutes();

src/dev/enola/be/task/TaskExecutorServices.java

@@ -0,0 +1,45 @@
+package dev.enola.common.concurrent;
+
+import static dev.enola.common.concurrent.LoggingThreadUncaughtExceptionHandler.toLogger;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.logging.Logger;
+
+public final class Executors {
+
+    public static ScheduledExecutorService newSingleThreadScheduledExecutor(
+            String namePrefix, Logger logger) {
+        var tf = createVirtualThreadFactory(namePrefix, logger);
+        return java.util.concurrent.Executors.newSingleThreadScheduledExecutor(tf);
+    }
+
+    public static ExecutorService
+            newVirtualThreadPerTaskExecutorWithLoggingThreadUncaughtExceptionHandler(
+                    Logger logger) {
+        var tf = createVirtualThreadFactory(logger);
+        return java.util.concurrent.Executors.newThreadPerTaskExecutor(tf);
+    }
+
+    private static ThreadFactory createVirtualThreadFactory(Logger logger) {
+        return Thread.ofVirtual()
+                .uncaughtExceptionHandler(toLogger(logger))
+                // NB: Virtual threads are always daemon threads, so no: .setDaemon(true)
+                // TODO new ContextAwareThreadFactory() how-to?
+                // TODO .inheritInheritableThreadLocals(true) ?
+                .factory();
+    }
+
+    private static ThreadFactory createVirtualThreadFactory(String namePrefix, Logger logger) {
+        return Thread.ofVirtual()
+                .name(namePrefix, 1)
+                .uncaughtExceptionHandler(toLogger(logger))
+                // NB: Virtual threads are always daemon threads, so no: .setDaemon(true)
+                // TODO new ContextAwareThreadFactory() how-to?
+                // TODO .inheritInheritableThreadLocals(true) ?
+                .factory();
+    }
+
+    private Executors() {}
+}

src/dev/enola/common/concurrent/Executors.java

@@ -0,0 +1,31 @@
+package dev.enola.common.concurrent;
+
+import static java.util.Objects.requireNonNull;
+
+import java.lang.Thread.UncaughtExceptionHandler;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+class LoggingThreadUncaughtExceptionHandler implements Thread.UncaughtExceptionHandler {
+
+    // Like dev.enola.common.concurrent.LoggingThreadUncaughtExceptionHandler in
+    // https://github.com/enola-dev/enola, but without the dependency on dev.enola.common and using
+    // JUL instead of SLF4J. TODO Maybe move this to dev.enola.common, later?
+
+    private final Logger logger;
+
+    private LoggingThreadUncaughtExceptionHandler(Logger logger) {
+        this.logger = requireNonNull(logger, "logger");
+    }
+
+    /** Factory method to obtain an instance of this bound to the passed JUL Logger. */
+    public static UncaughtExceptionHandler toLogger(Logger logger) {
+        return new LoggingThreadUncaughtExceptionHandler(logger);
+    }
+
+    @Override
+    public void uncaughtException(Thread thread, Throwable throwable) {
+        logger.log(
+                Level.SEVERE, "Uncaught exception in thread '" + thread.getName() + "'", throwable);
+    }
+}