Adds shutdown to Plugin

chunky/src/java/se/llbit/chunky/Plugin.java

@@ -18,6 +18,7 @@
 package se.llbit.chunky;
 
 import se.llbit.chunky.main.Chunky;
+import se.llbit.util.concurrent.ChunkyThread;
 
 /**
  * The plugin interface for Chunky plugins.
@@ -33,4 +34,17 @@ public interface Plugin {
    * @param chunky Chunky instance which the plugin should attach to
    */
   void attach(Chunky chunky);
+
+  /**
+   * Called when chunky shuts down, allowing the plugin to close critical resources. Most plugins do not need to do
+   * anything here.
+   *
+   * <p>This function will be called after all {@link ChunkyThread}s have shut down. Chunky's UI thread may still
+   * be running.</p>
+   * <p>This method will not be called if any {@link ChunkyThread} does not shut down within its given time limit</p>
+   * <p>This method may not be called if chunky terminates in a non-normal way, such as through {@link Runtime#halt},
+   * the user killing the process (<code>SIGKILL</code> & <code>TerminateProcess</code>), or an error within a native
+   * function.</p>
+   */
+  default void shutdown(Chunky chunky) { }
 }

chunky/src/java/se/llbit/chunky/main/Chunky.java

@@ -18,6 +18,7 @@
 package se.llbit.chunky.main;
 
 import se.llbit.chunky.PersistentSettings;
+import se.llbit.chunky.Plugin;
 import se.llbit.chunky.block.BlockProvider;
 import se.llbit.chunky.block.BlockSpec;
 import se.llbit.chunky.block.MinecraftBlockProvider;
@@ -48,6 +49,7 @@
 import se.llbit.log.Log;
 import se.llbit.log.Receiver;
 import se.llbit.util.TaskTracker;
+import se.llbit.util.concurrent.ChunkyThread;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -56,6 +58,7 @@
 import java.nio.file.Path;
 import java.util.*;
 import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
 /**
@@ -85,6 +88,7 @@ public void logEvent(Level level, String message) {
   };
 
   public final ChunkyOptions options;
+  private final PluginManager pluginManager;
   private RenderController renderController;
   private SceneFactory sceneFactory = SceneFactory.DEFAULT;
   private RenderContextFactory renderContextFactory = RenderContext::new;
@@ -106,6 +110,7 @@ public static String getMainWindowTitle() {
 
   public Chunky(ChunkyOptions options) {
     this.options = options;
+    this.pluginManager = new PluginManager(new JarPluginLoader());
     registerBlockProvider(new MinecraftBlockProvider());
     registerBlockProvider(new LegacyMinecraftBlockProvider());
   }
@@ -211,9 +216,8 @@ public static void main(final String[] args) {
       System.exit(1);
     }
 
-    int exitCode = 0;
     if (cmdline.mode == CommandLineOptions.Mode.CLI_OPERATION) {
-      exitCode = cmdline.exitCode;
+      System.exit(cmdline.exitCode);
     } else {
       // Initialize the common thread pool.
       getCommonThreads();
@@ -222,6 +226,14 @@ public static void main(final String[] args) {
       chunky.headless = cmdline.mode == Mode.HEADLESS_RENDER || cmdline.mode == Mode.CREATE_SNAPSHOT;
       chunky.loadPlugins();
 
+      Runtime.getRuntime().addShutdownHook(
+        new Thread(() -> { // intentionally not a ChunkyThread
+          // Within a shutdown hook we need to close quickly, otherwise we  risk the user/OS escalating to KILL
+          chunky.shutdown(1, TimeUnit.SECONDS);
+        })
+      );
+
+      int exitCode = 0;
       try {
         switch (cmdline.mode) {
           case HEADLESS_RENDER:
@@ -240,12 +252,21 @@ public static void main(final String[] args) {
         Log.error("Unchecked exception caused Chunky to close.", t);
         exitCode = 2;
       }
-    }
-    if (exitCode != 0) {
+      chunky.shutdown(5, TimeUnit.SECONDS);
+      // Always exit, we've done all shutdown necessary and want to exit whether non-daemon threads exist or not.
+      // This should prevent hangs if threads aren't cooperating.
       System.exit(exitCode);
     }
   }
 
+  private void shutdown(int timeout, TimeUnit unit) {
+    if (ChunkyThread.interruptAndJoinAll(timeout, unit)) {
+      pluginManager.shutdownPlugins(plugin -> plugin.shutdown(this));
+    } else {
+      Log.error("Not all threads were joined before shutting down."); // FIXME: list all alive threads? ThreadGroups are annoying.
+    }
+  }
+
   /**
    * This can be used by plugins to load the default Minecraft textures.
    */
@@ -255,40 +276,10 @@ public static void loadDefaultTextures() {
   }
 
   private void loadPlugins() {
-    File pluginsDirectory = SettingsDirectory.getPluginsDirectory();
-    if (!pluginsDirectory.isDirectory()) {
-      Log.infof("Plugins directory does not exist: %s", pluginsDirectory.getAbsolutePath());
-      return;
-    }
-    Path pluginsPath = pluginsDirectory.toPath();
-    JsonArray plugins = PersistentSettings.getPlugins();
-    // TODO: allow plugins to implement a custom plugin loader.
-    PluginManager pluginManager = new PluginManager(new JarPluginLoader());
-
-    // Parse plugin manifests
-    Set<PluginManifest> pluginManifests = plugins.elements.stream()
-      .map(value -> value.asString(""))
-      .filter(jarName -> !jarName.isEmpty())
-      .map(jarName -> pluginsPath.resolve(jarName).toAbsolutePath().toFile())
-      .map(PluginManager::parsePluginManifest)
-      .flatMap(Optional::stream)
-      .collect(Collectors.toSet());
-
-    // Load plugins
-    pluginManager.load(pluginManifests, (plugin, manifest) -> {
-      String jarName = manifest.pluginJar.getName();
-      Log.infof("Loading plugin: %s", jarName);
-      if (!isHeadless()) {
-        CreditsController.addPlugin(manifest.name, manifest.version.toString(), manifest.author, manifest.description);
-      }
-
-      try {
-        plugin.attach(this);
-      } catch (Throwable t) {
-        Log.error("Plugin " + jarName + " failed to load.", t);
-      }
-      Log.infof("Plugin loaded: %s %s", manifest.name, manifest.version);
-    });
+    this.pluginManager.loadPluginsFromDirectory(
+      SettingsDirectory.getPluginsDirectory(),
+      (plugin, manifest) -> plugin.attach(this)
+    );
   }
 
   /**
@@ -342,7 +333,7 @@ public void update() {
   public static ForkJoinPool getCommonThreads() {
     if (commonThreads == null) {
       // use at least two threads to prevent deadlocks in some java versions (see #1631)
-      commonThreads = new ForkJoinPool(Math.max(PersistentSettings.getNumThreads(), 2));
+      commonThreads = ChunkyThread.addForkJoinPool(new ForkJoinPool(Math.max(PersistentSettings.getNumThreads(), 2)));
     }
     return commonThreads;
   }
@@ -353,7 +344,7 @@ public static ForkJoinPool getCommonThreads() {
    */
   public static void setCommonThreadsCount(int threads) {
     ForkJoinPool t = getCommonThreads();
-    commonThreads = new ForkJoinPool(threads);
+    commonThreads = ChunkyThread.addForkJoinPool(new ForkJoinPool(threads));
     t.shutdown();
   }
 

chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java

@@ -16,9 +16,11 @@
  */
 package se.llbit.chunky.plugin.loader;
 
+import se.llbit.chunky.PersistentSettings;
 import se.llbit.chunky.Plugin;
 import se.llbit.chunky.plugin.manifest.PluginDependency;
 import se.llbit.chunky.plugin.manifest.PluginManifest;
+import se.llbit.json.JsonArray;
 import se.llbit.json.JsonParser;
 import se.llbit.log.Log;
 
@@ -31,19 +33,75 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.*;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiConsumer;
+import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 public class PluginManager {
-  private static final int MAX_CYCLES = Integer.parseInt(System.getProperty("chunky.plugins.maxLoadCycles", "100"));
-
   private final PluginLoader pluginLoader;
+  private final LinkedList<PluginEntry> plugins = new LinkedList<>();
+  private final AtomicBoolean isShutdown = new AtomicBoolean(false);
 
   public PluginManager(PluginLoader pluginLoader) {
     this.pluginLoader = pluginLoader;
   }
 
-  public void load(Set<PluginManifest> pluginManifests, BiConsumer<Plugin, PluginManifest> onLoad) {
+  public void loadPluginsFromDirectory(File pluginsDirectory, BiConsumer<Plugin, PluginManifest> onLoad) {
+    if (!pluginsDirectory.isDirectory()) {
+      Log.infof("Plugins directory does not exist: %s", pluginsDirectory.getAbsolutePath());
+      return;
+    }
+    Path pluginsPath = pluginsDirectory.toPath();
+    JsonArray plugins = PersistentSettings.getPlugins();
+    // TODO: allow plugins to implement a custom plugin loader?.
+
+    // Parse plugin manifests
+    Set<PluginManifest> pluginManifests = plugins.elements.stream()
+      .map(value -> value.asString(""))
+      .filter(jarName -> !jarName.isEmpty())
+      .map(jarName -> pluginsPath.resolve(jarName).toAbsolutePath().toFile())
+      .map(PluginManager::parsePluginManifest)
+      .flatMap(Optional::stream)
+      .collect(Collectors.toSet());
+
+    loadPlugins(pluginManifests, onLoad);
+  }
+
+  public void loadPlugins(Set<PluginManifest> pluginManifests, BiConsumer<Plugin, PluginManifest> onLoad) {
+    // Load plugins
+    this.load(pluginManifests, (plugin, manifest) -> {
+      String jarName = manifest.pluginJar.getName();
+      Log.infof("Loading plugin: %s", jarName);
+      try {
+        onLoad.accept(plugin, manifest);
+      } catch (Throwable t) {
+        Log.error("Plugin " + jarName + " failed to load.", t);
+      }
+      Log.infof("Plugin loaded: %s %s", manifest.name, manifest.version);
+    });
+  }
+
+  /**
+   * Calls shutdown on every plugin.
+   * <p>Has no effect if already shutdown.</p>
+   */
+  public void shutdownPlugins(Consumer<Plugin> onShutdown) {
+    if (this.isShutdown.getAndSet(true)) {
+      return;
+    }
+
+    // Iterates backwards to shut down plugins in the reverse of the order they were attached
+    this.plugins.descendingIterator().forEachRemaining(plugin -> {
+      try {
+        onShutdown.accept(plugin.plugin);
+      } catch (Throwable t) {
+        Log.error(String.format("The plugin %s threw when shutting down", plugin.manifest.name), t);
+      }
+    });
+  }
+
+  private void load(Set<PluginManifest> pluginManifests, BiConsumer<Plugin, PluginManifest> onLoad) {
     // create plugin objects
     Map<String, List<ResolvedPlugin>> pluginsByName = new HashMap<>();
     pluginManifests.forEach(manifest -> {
@@ -64,33 +122,38 @@ public void load(Set<PluginManifest> pluginManifests, BiConsumer<Plugin, PluginM
     Set<ResolvedPlugin> pluginsToLoad = pluginsByName.values().stream().flatMap(Collection::stream).collect(Collectors.toSet());
     pluginsToLoad.forEach(plugin -> plugin.resolveDependencies(pluginsByName));
 
-    // load plugins in dependency-first order, cyclic dependencies will never be loaded and will hit MAX_CYCLES cap.
-    // this was so trivial to implement using cycles that I decided against any kind of dependency tree structure,
-    // in the worst case this approach requires one cycle per plugin (if every plugin depended on the previous one in the list).
+    int maxCycles = pluginsToLoad.size();
+    // Load plugins in dependency-first order, cyclic dependencies will never be loaded and will hit the maxCycles cap.
+    // This was so trivial to implement using cycles that I decided against any kind of dependency tree structure.
+    // In the worst case this approach requires one cycle per plugin (if every plugin depended on the previous one in the list).
     Set<ResolvedPlugin> loadedPlugins = new HashSet<>();
     int loadCycles = 0;
-    while (!pluginsToLoad.isEmpty() && loadCycles < MAX_CYCLES) {
+    while (!pluginsToLoad.isEmpty() && loadCycles < maxCycles) {
       Log.infof("Cycle %d", loadCycles);
       loadCycles++;
-      // new list to avoid CME due to removing inside iteration.
-      new ArrayList<>(pluginsToLoad).forEach(plugin -> {
+      for (Iterator<ResolvedPlugin> iterator = pluginsToLoad.iterator(); iterator.hasNext(); ) {
+        ResolvedPlugin plugin = iterator.next();
         if (plugin.allDependenciesLoaded(loadedPlugins)) {
           loadedPlugins.add(plugin);
-          pluginsToLoad.remove(plugin);
+          iterator.remove();
           Log.infof("  Loading plugin %s with deps { %s }, resolved { %s }%n", plugin,
             plugin.getManifest().getDependencies().stream().map(PluginDependency::toString).collect(Collectors.joining(", ")),
             plugin.getDependencies().stream().map(ResolvedPlugin::toString).collect(Collectors.joining(", "))
           );
-          pluginLoader.load(onLoad, plugin.getManifest());
+          pluginLoader.load((loadedPlugin, manifest) -> {
+            this.plugins.add(new PluginEntry(manifest, loadedPlugin));
+            onLoad.accept(loadedPlugin, manifest);
+          }, plugin.getManifest());
         }
-      });
+      }
     }
 
     // report if any unloaded plugins remain (their dependencies never got loaded)
     if (!pluginsToLoad.isEmpty()) {
       Log.errorf(
-        "Reached maximum cycles (%d) when loading plugins. Failed to load plugins: (%s)",
-        MAX_CYCLES,
+        "Reached maximum cycles %d when loading %d plugins. Failed to load plugins: (%s)",
+        maxCycles,
+        pluginManifests.size(),
         pluginsToLoad.stream().map(ResolvedPlugin::toString).collect(Collectors.joining(", "))
       );
     }
@@ -118,4 +181,6 @@ public static Optional<PluginManifest> parsePluginManifest(File pluginJar) {
     }
     return Optional.empty();
   }
+
+  private record PluginEntry(PluginManifest manifest, Plugin plugin) {}
 }

chunky/src/java/se/llbit/chunky/renderer/DefaultRenderManager.java

@@ -28,6 +28,7 @@
 import se.llbit.chunky.resources.BitmapImage;
 import se.llbit.log.Log;
 import se.llbit.math.ColorUtil;
+import se.llbit.util.concurrent.ChunkyThread;
 import se.llbit.util.TaskTracker;
 
 import java.time.Duration;
@@ -52,7 +53,7 @@
  * <p>All available final renderers are stored in {@code renderers} and preview renderers
  * are stored in {@code previewRenderers}.
  */
-public class DefaultRenderManager extends Thread implements RenderManager {
+public class DefaultRenderManager extends ChunkyThread implements RenderManager {
   /**
    * Map containing all the final render {@code Renderer}s. The renderer corresponding to
    * {@code getRendererName()} is used when a render is requested.

chunky/src/java/se/llbit/chunky/renderer/RenderWorkerPool.java

@@ -18,6 +18,7 @@
 package se.llbit.chunky.renderer;
 
 import se.llbit.log.Log;
+import se.llbit.util.concurrent.ChunkyThread;
 
 import java.util.ArrayList;
 import java.util.Random;
@@ -39,7 +40,7 @@ public interface Factory {
     RenderWorkerPool create(int threads, long seed);
   }
 
-  public static class RenderWorker extends Thread {
+  public static class RenderWorker extends ChunkyThread {
     private final RenderWorkerPool pool;
 
     public final Random random;

chunky/src/java/se/llbit/chunky/renderer/scene/AsynchronousSceneManager.java

@@ -26,6 +26,7 @@
 import se.llbit.chunky.world.RegionPosition;
 import se.llbit.chunky.world.World;
 import se.llbit.log.Log;
+import se.llbit.util.concurrent.ChunkyThread;
 import se.llbit.util.TaskTracker;
 
 import java.io.File;
@@ -41,7 +42,7 @@
  *
  * @author Jesper Öqvist <jesper@llbit.se>
  */
-public class AsynchronousSceneManager extends Thread implements SceneManager {
+public class AsynchronousSceneManager extends ChunkyThread implements SceneManager {
 
   private final SynchronousSceneManager sceneManager;
   private final LinkedBlockingQueue<Runnable> taskQueue;

chunky/src/java/se/llbit/chunky/renderer/scene/Scene.java

@@ -62,6 +62,7 @@
 import se.llbit.nbt.Tag;
 import se.llbit.util.*;
 import se.llbit.util.annotation.NotNull;
+import se.llbit.util.concurrent.ChunkyThread;
 import se.llbit.util.io.PositionalInputStream;
 import se.llbit.util.io.ZipExport;
 import se.llbit.util.mojangapi.MinecraftProfile;
@@ -858,7 +859,7 @@ public synchronized void loadChunks(TaskTracker taskTracker, World world, Map<Re
       int[] cubeWorldBlocks = new int[16*16*16];
       int[] cubeWaterBlocks = new int[16*16*16];
 
-      ExecutorService executor = Executors.newSingleThreadExecutor();
+      ExecutorService executor = ChunkyThread.addExecutorService(Executors::newSingleThreadExecutor);
 
       ChunkData[] regionParsingDataArray = new ChunkData[MCRegion.CHUNKS_X * MCRegion.CHUNKS_Z];
       ChunkData[] chunkLoadingDataArray = new ChunkData[MCRegion.CHUNKS_X * MCRegion.CHUNKS_Z];