From 133ee5224c272278b9a1398648bc82ac7fe5b3cd Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Wed, 8 Jul 2026 12:47:01 +0100 Subject: [PATCH 01/17] Create a single place for all chunky thread/executors to be registered Closes #1893 - Allows chunky to close them on shutdown without a System.exit() - Additionally provides an api to wait on all chunky threads to be joined. --- .../src/java/se/llbit/chunky/main/Chunky.java | 7 + .../chunky/renderer/DefaultRenderManager.java | 3 +- .../chunky/renderer/RenderWorkerPool.java | 3 +- .../scene/AsynchronousSceneManager.java | 3 +- .../se/llbit/chunky/renderer/scene/Scene.java | 3 +- .../src/java/se/llbit/chunky/ui/ChunkMap.java | 3 +- .../src/java/se/llbit/chunky/ui/ChunkyFx.java | 1 - .../ResourcePackChooserController.java | 3 +- .../ui/controller/SceneChooserController.java | 4 +- .../chunky/world/ChunkTopographyUpdater.java | 4 +- .../se/llbit/chunky/world/SkymapTexture.java | 3 +- .../world/region/RegionChangeWatcher.java | 3 +- .../chunky/world/region/RegionParser.java | 3 +- .../llbit/util/concurrent/ChunkyThread.java | 169 ++++++++++++++++++ 14 files changed, 200 insertions(+), 12 deletions(-) create mode 100644 chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java diff --git a/chunky/src/java/se/llbit/chunky/main/Chunky.java b/chunky/src/java/se/llbit/chunky/main/Chunky.java index 13a6965357..976a5038cd 100644 --- a/chunky/src/java/se/llbit/chunky/main/Chunky.java +++ b/chunky/src/java/se/llbit/chunky/main/Chunky.java @@ -48,6 +48,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; @@ -241,6 +242,12 @@ public static void main(final String[] args) { exitCode = 2; } } + + ChunkyThread.interruptAndJoinAll(); + ForkJoinPool commonThreads = Chunky.getCommonThreads(); + commonThreads.shutdownNow(); // ForkJoinPool doesn't return any tasks that were awaiting execution (all canceled). + // We don't use the ForkJoinPool commonPool in chunky, but if we did there is no shutdown available so nothing changes. + if (exitCode != 0) { System.exit(exitCode); } diff --git a/chunky/src/java/se/llbit/chunky/renderer/DefaultRenderManager.java b/chunky/src/java/se/llbit/chunky/renderer/DefaultRenderManager.java index 9b091c2e43..6074c35775 100644 --- a/chunky/src/java/se/llbit/chunky/renderer/DefaultRenderManager.java +++ b/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 @@ *

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. diff --git a/chunky/src/java/se/llbit/chunky/renderer/RenderWorkerPool.java b/chunky/src/java/se/llbit/chunky/renderer/RenderWorkerPool.java index 51b129d5d8..80cef5ea46 100644 --- a/chunky/src/java/se/llbit/chunky/renderer/RenderWorkerPool.java +++ b/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; diff --git a/chunky/src/java/se/llbit/chunky/renderer/scene/AsynchronousSceneManager.java b/chunky/src/java/se/llbit/chunky/renderer/scene/AsynchronousSceneManager.java index 919a4f4c2f..7d08780263 100644 --- a/chunky/src/java/se/llbit/chunky/renderer/scene/AsynchronousSceneManager.java +++ b/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 */ -public class AsynchronousSceneManager extends Thread implements SceneManager { +public class AsynchronousSceneManager extends ChunkyThread implements SceneManager { private final SynchronousSceneManager sceneManager; private final LinkedBlockingQueue taskQueue; diff --git a/chunky/src/java/se/llbit/chunky/renderer/scene/Scene.java b/chunky/src/java/se/llbit/chunky/renderer/scene/Scene.java index bc4bc6bf61..82d0f3f21a 100644 --- a/chunky/src/java/se/llbit/chunky/renderer/scene/Scene.java +++ b/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 {}; - private ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(); + private final ScheduledExecutorService executor = ChunkyThread.addExecutorService(Executors::newSingleThreadScheduledExecutor); private boolean shouldDrawPlayers = true; diff --git a/chunky/src/java/se/llbit/chunky/ui/ChunkyFx.java b/chunky/src/java/se/llbit/chunky/ui/ChunkyFx.java index 93f6532acc..44a552b5f4 100644 --- a/chunky/src/java/se/llbit/chunky/ui/ChunkyFx.java +++ b/chunky/src/java/se/llbit/chunky/ui/ChunkyFx.java @@ -82,7 +82,6 @@ public void stop() throws Exception { if(mainStage != null) { PersistentSettings.setWindowPosition(new WindowPosition(mainStage)); } - System.exit(0); } public static void startChunkyUI(Chunky chunkyInstance) { diff --git a/chunky/src/java/se/llbit/chunky/ui/controller/ResourcePackChooserController.java b/chunky/src/java/se/llbit/chunky/ui/controller/ResourcePackChooserController.java index dcac327038..251fdff9d5 100644 --- a/chunky/src/java/se/llbit/chunky/ui/controller/ResourcePackChooserController.java +++ b/chunky/src/java/se/llbit/chunky/ui/controller/ResourcePackChooserController.java @@ -49,6 +49,7 @@ import se.llbit.json.JsonParser; import se.llbit.log.Log; import se.llbit.util.MinecraftText; +import se.llbit.util.concurrent.ChunkyThread; import java.awt.*; import java.io.File; @@ -451,7 +452,7 @@ public void populate( private static class PackListItem { - private final static Executor PACK_PARSER_EXECUTOR = Executors.newSingleThreadExecutor(); + private final static Executor PACK_PARSER_EXECUTOR = ChunkyThread.addExecutorService(Executors::newSingleThreadExecutor); private static PackListItem DEFAULT = null; diff --git a/chunky/src/java/se/llbit/chunky/ui/controller/SceneChooserController.java b/chunky/src/java/se/llbit/chunky/ui/controller/SceneChooserController.java index 534926aeac..5c507b0568 100644 --- a/chunky/src/java/se/llbit/chunky/ui/controller/SceneChooserController.java +++ b/chunky/src/java/se/llbit/chunky/ui/controller/SceneChooserController.java @@ -36,6 +36,7 @@ import se.llbit.json.JsonObject; import se.llbit.json.JsonParser; import se.llbit.log.Log; +import se.llbit.util.concurrent.ChunkyThread; import java.io.File; import java.io.FileInputStream; @@ -64,6 +65,8 @@ public class SceneChooserController implements Initializable { private Stage stage; + private static final Executor loadExecutor = ChunkyThread.addExecutorService(Executors::newSingleThreadExecutor); + private ChunkyFxController controller; private static final HashMap sceneListCache = new HashMap<>(); @@ -219,7 +222,6 @@ public void setStage(Stage stage) { private void populateSceneTable(File sceneDir) { this.sceneTbl.setPlaceholder(new Label("Loading scenes…")); - Executor loadExecutor = Executors.newSingleThreadExecutor(); loadExecutor.execute(() -> { List scenes = new ArrayList<>(); diff --git a/chunky/src/java/se/llbit/chunky/world/ChunkTopographyUpdater.java b/chunky/src/java/se/llbit/chunky/world/ChunkTopographyUpdater.java index 8c61d5b591..58ef6be998 100644 --- a/chunky/src/java/se/llbit/chunky/world/ChunkTopographyUpdater.java +++ b/chunky/src/java/se/llbit/chunky/world/ChunkTopographyUpdater.java @@ -16,6 +16,8 @@ */ package se.llbit.chunky.world; +import se.llbit.util.concurrent.ChunkyThread; + import java.util.HashSet; import java.util.Iterator; import java.util.Set; @@ -25,7 +27,7 @@ * * @author Jesper Öqvist (jesper@llbit.se) */ -public class ChunkTopographyUpdater extends Thread { +public class ChunkTopographyUpdater extends ChunkyThread { private final Set queue = new HashSet<>(); diff --git a/chunky/src/java/se/llbit/chunky/world/SkymapTexture.java b/chunky/src/java/se/llbit/chunky/world/SkymapTexture.java index 4b2500a901..d879dc1576 100644 --- a/chunky/src/java/se/llbit/chunky/world/SkymapTexture.java +++ b/chunky/src/java/se/llbit/chunky/world/SkymapTexture.java @@ -25,6 +25,7 @@ import se.llbit.math.QuickMath; import se.llbit.math.Ray; import se.llbit.math.Vector4; +import se.llbit.util.concurrent.ChunkyThread; import se.llbit.util.ImageTools; /** @@ -35,7 +36,7 @@ */ public class SkymapTexture extends Texture { - class TexturePreprocessor extends Thread { + class TexturePreprocessor extends ChunkyThread { private final int x0; private final int x1; private final int y0; diff --git a/chunky/src/java/se/llbit/chunky/world/region/RegionChangeWatcher.java b/chunky/src/java/se/llbit/chunky/world/region/RegionChangeWatcher.java index 01ab9ed1b2..bdc35cd969 100644 --- a/chunky/src/java/se/llbit/chunky/world/region/RegionChangeWatcher.java +++ b/chunky/src/java/se/llbit/chunky/world/region/RegionChangeWatcher.java @@ -20,13 +20,14 @@ import se.llbit.chunky.map.WorldMapLoader; import se.llbit.chunky.renderer.ChunkViewListener; import se.llbit.chunky.world.ChunkView; +import se.llbit.util.concurrent.ChunkyThread; /** * Monitors filesystem for changes to region files. * * @author Jesper Öqvist */ -public abstract class RegionChangeWatcher extends Thread implements ChunkViewListener { +public abstract class RegionChangeWatcher extends ChunkyThread implements ChunkViewListener { protected final WorldMapLoader mapLoader; protected final MapView mapView; protected volatile ChunkView view = ChunkView.EMPTY; diff --git a/chunky/src/java/se/llbit/chunky/world/region/RegionParser.java b/chunky/src/java/se/llbit/chunky/world/region/RegionParser.java index b25f0771c0..841851a51e 100644 --- a/chunky/src/java/se/llbit/chunky/world/region/RegionParser.java +++ b/chunky/src/java/se/llbit/chunky/world/region/RegionParser.java @@ -23,6 +23,7 @@ import se.llbit.chunky.map.WorldMapLoader; import se.llbit.chunky.world.*; import se.llbit.log.Log; +import se.llbit.util.concurrent.ChunkyThread; import se.llbit.util.Mutable; /** @@ -34,7 +35,7 @@ * * @author Jesper Öqvist (jesper@llbit.se) */ -public class RegionParser extends Thread { +public class RegionParser extends ChunkyThread { private final WorldMapLoader mapLoader; private final RegionQueue queue; diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java new file mode 100644 index 0000000000..1ba27c88cb --- /dev/null +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -0,0 +1,169 @@ +package se.llbit.util.concurrent; + +import se.llbit.chunky.main.Chunky; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; + +/** + * {@link Thread}/{@link ExecutorService} resource management. + *

The goal of this glass is to primarily:

+ * + * + *

Usage

+ *

{@link Thread Threads} in chunky should extend this class, and {@link ExecutorService executor services} should + * be created with {@link #addExecutorService(Function)} to allow chunky to interrupt and join them before chunky closes.

+ * + */ +public class ChunkyThread extends Thread { + /* + * All operations lock. + * When interruptAndJoinAll is called, additional threads/executors can't be added preventing later joins from + * waiting on threads that have not been interrupted. + */ + private static final AtomicBoolean isShutdown = new AtomicBoolean(false); + private static final Collection threads = new ArrayList<>(); + private static final Collection executorServices = new ArrayList<>(); + + /** + * Add a {@link Thread} to be interrupted and joined by chunky on shutdown + * + * @throws IllegalStateException When calling after {@link #interruptAndJoinAll()} has been called. + */ + public synchronized static T addThread(T thread) { + if (isShutdown.get()) { + throw new IllegalStateException("Creating a thread as chunky is stopping."); + } + threads.add(thread); + return thread; + } + + /** + * Add an {@link ExecutorService} to be interrupted and joined by chunky on shutdown + * + @throws IllegalStateException When calling after {@link #interruptAndJoinAll()} has been called. + */ + public synchronized static E addExecutorService(Function executorServiceSupplier) { + if (isShutdown.get()) { + throw new IllegalStateException("Creating an executor service as chunky is stopping."); + } + E e = executorServiceSupplier.apply(ChunkyThread::new); + executorServices.add(e); + return e; + } + + /** + * Await the joining of all threads managed by chunky + * + *

This method is always safe to call.

+ * + *

WARNING: calling this from any thread registered with {@link #addThread(Thread)} may deadlock.

+ */ + public synchronized static void joinAll() { + for (Thread thread : threads) { + try { + thread.join(); + } catch (InterruptedException e) { + // ignored + } + } + for (ExecutorService executorService : executorServices) { + try { + executorService.awaitTermination(1, TimeUnit.MINUTES); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + + /** + * Interrupt and then await joining of all threads managed by chunky. + * + *

This method is always safe to call.

+ * + *

WARNING: calling this from any thread registered with {@link #addThread(Thread)} may deadlock.

+ *

Only to be called by {@link Chunky}

+ */ + public synchronized static void interruptAndJoinAll() { + assert Thread.currentThread().getName().equals("main"); + isShutdown.set(true); + + // shut down executorServices BEFORE threads because they recreate their threads when they are interrupted and stop + // causing an infinite hang. + for (ExecutorService executorService : executorServices) { + executorService.shutdownNow(); + } + + for (Thread thread : ChunkyThread.threads) { + thread.interrupt(); + } + for (Thread thread : ChunkyThread.threads) { + try { + thread.join(); + } catch (InterruptedException e) { + // ignored + } + } + } + + private void setDefaults() { + this.setDaemon(true); + addThread(this); + } + + /* Constructors from super */ + public ChunkyThread() { + super(); + setDefaults(); + } + + public ChunkyThread(Runnable task) { + super(task); + setDefaults(); + } + + public ChunkyThread(ThreadGroup group, Runnable task) { + super(group, task); + setDefaults(); + } + + public ChunkyThread(String name) { + super(name); + setDefaults(); + } + + public ChunkyThread(ThreadGroup group, String name) { + super(group, name); + setDefaults(); + } + + public ChunkyThread(Runnable task, String name) { + super(task, name); + setDefaults(); + } + + public ChunkyThread(ThreadGroup group, Runnable task, String name) { + super(group, task, name); + setDefaults(); + } + + public ChunkyThread(ThreadGroup group, Runnable task, String name, long stackSize) { + super(group, task, name, stackSize); + setDefaults(); + } + + public ChunkyThread(ThreadGroup group, Runnable task, String name, long stackSize, boolean inheritInheritableThreadLocals) { + super(group, task, name, stackSize, inheritInheritableThreadLocals); + setDefaults(); + } +} From 30f3aa033ff9aba03ff7126350ecd8c47f56d17b Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Sat, 11 Jul 2026 09:51:07 +0100 Subject: [PATCH 02/17] Fix race between joinAll and extremely lately added threads/executors --- .../llbit/util/concurrent/ChunkyThread.java | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index 1ba27c88cb..066efc9aae 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -1,13 +1,14 @@ package se.llbit.util.concurrent; import se.llbit.chunky.main.Chunky; +import se.llbit.log.Log; import java.util.ArrayList; import java.util.Collection; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; /** @@ -31,7 +32,7 @@ public class ChunkyThread extends Thread { * When interruptAndJoinAll is called, additional threads/executors can't be added preventing later joins from * waiting on threads that have not been interrupted. */ - private static final AtomicBoolean isShutdown = new AtomicBoolean(false); + private static final CountDownLatch shutdownLatch = new CountDownLatch(1); private static final Collection threads = new ArrayList<>(); private static final Collection executorServices = new ArrayList<>(); @@ -41,7 +42,7 @@ public class ChunkyThread extends Thread { * @throws IllegalStateException When calling after {@link #interruptAndJoinAll()} has been called. */ public synchronized static T addThread(T thread) { - if (isShutdown.get()) { + if (shutdownLatch.getCount() == 0) { throw new IllegalStateException("Creating a thread as chunky is stopping."); } threads.add(thread); @@ -54,7 +55,7 @@ public synchronized static T addThread(T thread) { @throws IllegalStateException When calling after {@link #interruptAndJoinAll()} has been called. */ public synchronized static E addExecutorService(Function executorServiceSupplier) { - if (isShutdown.get()) { + if (shutdownLatch.getCount() == 0) { throw new IllegalStateException("Creating an executor service as chunky is stopping."); } E e = executorServiceSupplier.apply(ChunkyThread::new); @@ -70,20 +71,37 @@ public synchronized static E addExecutorService(Func *

WARNING: calling this from any thread registered with {@link #addThread(Thread)} may deadlock.

*/ public synchronized static void joinAll() { + boolean interrupted = false; + + while (true) { + try { + // must wait for the latch as hitting the for loop below first causes immediate evaluation of the + // for loop iterator, potentially missing new threads. + shutdownLatch.await(); + break; + } catch (InterruptedException e) { + interrupted = true; + } + } + for (Thread thread : threads) { try { thread.join(); } catch (InterruptedException e) { - // ignored + interrupted = true; } } for (ExecutorService executorService : executorServices) { try { executorService.awaitTermination(1, TimeUnit.MINUTES); } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + interrupted = true; } } + + if (interrupted) { + Thread.currentThread().interrupt(); + } } /** @@ -96,7 +114,7 @@ public synchronized static void joinAll() { */ public synchronized static void interruptAndJoinAll() { assert Thread.currentThread().getName().equals("main"); - isShutdown.set(true); + shutdownLatch.countDown(); // shut down executorServices BEFORE threads because they recreate their threads when they are interrupted and stop // causing an infinite hang. From c675acea49500b77020865b442653b0094471c09 Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Mon, 13 Jul 2026 08:03:18 +0100 Subject: [PATCH 03/17] Make joinAll not synchronized --- .../src/java/se/llbit/util/concurrent/ChunkyThread.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index 066efc9aae..d10300097a 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -70,7 +70,13 @@ public synchronized static E addExecutorService(Func * *

WARNING: calling this from any thread registered with {@link #addThread(Thread)} may deadlock.

*/ - public synchronized static void joinAll() { + public static void joinAll() { + /* + * This method should not be synchronized because: + * 1. Calls to this method that happen before interruptAndJoinAll will lock the latter interrupting thread, deadlocking. + * 2. shutdownLatch.await is at least acquire memory ordering, and modification is disabled after the latch is zero. + * As such we are guaranteed that no threads can modify the state. + */ boolean interrupted = false; while (true) { From 1d166b1c5ce9e05959ff2d768892f6842fc300c7 Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Mon, 13 Jul 2026 08:49:26 +0100 Subject: [PATCH 04/17] Return whether all threads were joined from joinAll methods --- .../src/java/se/llbit/chunky/main/Chunky.java | 3 +- .../llbit/util/concurrent/ChunkyThread.java | 116 ++++++++++-------- 2 files changed, 68 insertions(+), 51 deletions(-) diff --git a/chunky/src/java/se/llbit/chunky/main/Chunky.java b/chunky/src/java/se/llbit/chunky/main/Chunky.java index 976a5038cd..4888adcb9a 100644 --- a/chunky/src/java/se/llbit/chunky/main/Chunky.java +++ b/chunky/src/java/se/llbit/chunky/main/Chunky.java @@ -57,6 +57,7 @@ import java.nio.file.Path; import java.util.*; import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; /** @@ -243,7 +244,7 @@ public static void main(final String[] args) { } } - ChunkyThread.interruptAndJoinAll(); + ChunkyThread.interruptAndJoinAll(5, TimeUnit.SECONDS); ForkJoinPool commonThreads = Chunky.getCommonThreads(); commonThreads.shutdownNow(); // ForkJoinPool doesn't return any tasks that were awaiting execution (all canceled). // We don't use the ForkJoinPool commonPool in chunky, but if we did there is no shutdown available so nothing changes. diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index d10300097a..3284a7302d 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -1,19 +1,16 @@ package se.llbit.util.concurrent; import se.llbit.chunky.main.Chunky; -import se.llbit.log.Log; +import se.llbit.util.annotation.NotNull; import java.util.ArrayList; import java.util.Collection; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.*; import java.util.function.Function; /** * {@link Thread}/{@link ExecutorService} resource management. - *

The goal of this glass is to primarily:

+ *

The goal of this class is to primarily:

*
    *
  • Ensure all chunky threads (even daemons) are interrupted and given a chance clean up before getting killed by * the runtime on termination.
    • @@ -27,11 +24,6 @@ * */ public class ChunkyThread extends Thread { - /* - * All operations lock. - * When interruptAndJoinAll is called, additional threads/executors can't be added preventing later joins from - * waiting on threads that have not been interrupted. - */ private static final CountDownLatch shutdownLatch = new CountDownLatch(1); private static final Collection threads = new ArrayList<>(); private static final Collection executorServices = new ArrayList<>(); @@ -39,7 +31,7 @@ public class ChunkyThread extends Thread { /** * Add a {@link Thread} to be interrupted and joined by chunky on shutdown * - * @throws IllegalStateException When calling after {@link #interruptAndJoinAll()} has been called. + * @throws IllegalStateException When calling after {@link #interruptAndJoinAll(long, TimeUnit)} has been called. */ public synchronized static T addThread(T thread) { if (shutdownLatch.getCount() == 0) { @@ -52,7 +44,7 @@ public synchronized static T addThread(T thread) { /** * Add an {@link ExecutorService} to be interrupted and joined by chunky on shutdown * - @throws IllegalStateException When calling after {@link #interruptAndJoinAll()} has been called. + * @throws IllegalStateException When calling after {@link #interruptAndJoinAll(long, TimeUnit)} has been called. */ public synchronized static E addExecutorService(Function executorServiceSupplier) { if (shutdownLatch.getCount() == 0) { @@ -64,13 +56,18 @@ public synchronized static E addExecutorService(Func } /** - * Await the joining of all threads managed by chunky + * Await the joining of all threads managed by chunky. This method will wait indefinitely until a + * shutdown happens to begin its timeout. * *

    This method is always safe to call.

    * *

    WARNING: calling this from any thread registered with {@link #addThread(Thread)} may deadlock.

    + * + * @param timeout The maximum time to wait AFTER a shutdown is initiated + * @param unit the time unit of the timeout argument + * @return Whether all threads were joined before returning */ - public static void joinAll() { + public static boolean joinAll(long timeout, @NotNull TimeUnit unit) { /* * This method should not be synchronized because: * 1. Calls to this method that happen before interruptAndJoinAll will lock the latter interrupting thread, deadlocking. @@ -90,54 +87,73 @@ public static void joinAll() { } } - for (Thread thread : threads) { - try { - thread.join(); - } catch (InterruptedException e) { - interrupted = true; + long startTime = System.nanoTime(); + long endTime = startTime + unit.toNanos(timeout); + + boolean anyAlive = false; + + try { + for (ExecutorService executorService : executorServices) { + while (System.nanoTime() < endTime) { + try { + long waitTime = endTime - startTime; + if (waitTime > 0) { + executorService.awaitTermination(waitTime, TimeUnit.NANOSECONDS); + } + break; + } catch (InterruptedException e) { + interrupted = true; + } + } + anyAlive |= !executorService.isTerminated(); } - } - for (ExecutorService executorService : executorServices) { - try { - executorService.awaitTermination(1, TimeUnit.MINUTES); - } catch (InterruptedException e) { - interrupted = true; + for (Thread thread : ChunkyThread.threads) { + while (System.nanoTime() < endTime) { + try { + long waitTimeMillis = TimeUnit.NANOSECONDS.toMillis(endTime - startTime); + if (waitTimeMillis > 0) { + thread.join(waitTimeMillis); + } + break; + } catch (InterruptedException e) { + interrupted = true; + } + } + anyAlive |= thread.isAlive(); + } + } finally { + if (interrupted) { + Thread.currentThread().interrupt(); } } - - if (interrupted) { - Thread.currentThread().interrupt(); - } + return !anyAlive; } /** * Interrupt and then await joining of all threads managed by chunky. * - *

    This method is always safe to call.

    - * *

    WARNING: calling this from any thread registered with {@link #addThread(Thread)} may deadlock.

    *

    Only to be called by {@link Chunky}

    + * + * @param timeout The maximum time to wait + * @param unit the time unit of the timeout argument + * @return Whether all threads were joined before the time limit was reached */ - public synchronized static void interruptAndJoinAll() { - assert Thread.currentThread().getName().equals("main"); - shutdownLatch.countDown(); - - // shut down executorServices BEFORE threads because they recreate their threads when they are interrupted and stop - // causing an infinite hang. - for (ExecutorService executorService : executorServices) { - executorService.shutdownNow(); - } - - for (Thread thread : ChunkyThread.threads) { - thread.interrupt(); - } - for (Thread thread : ChunkyThread.threads) { - try { - thread.join(); - } catch (InterruptedException e) { - // ignored + public static boolean interruptAndJoinAll(long timeout, @NotNull TimeUnit unit) { + synchronized (ChunkyThread.class) { + shutdownLatch.countDown(); + + // shut down executorServices BEFORE threads because they recreate their threads when they are interrupted and stop + // causing an infinite hang. + for (ExecutorService executorService : executorServices) { + executorService.shutdownNow(); + } + for (Thread thread : ChunkyThread.threads) { + thread.interrupt(); } } + + return joinAll(timeout, unit); } private void setDefaults() { From 63367bec09dda00d0f8714e41d012b35ad433447 Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Mon, 13 Jul 2026 08:55:51 +0100 Subject: [PATCH 05/17] Add Chunky's ForkJoinPool to ChunkyThread --- .../src/java/se/llbit/chunky/main/Chunky.java | 7 ++----- .../llbit/util/concurrent/ChunkyThread.java | 20 +++++++++++++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/chunky/src/java/se/llbit/chunky/main/Chunky.java b/chunky/src/java/se/llbit/chunky/main/Chunky.java index 4888adcb9a..14e0925915 100644 --- a/chunky/src/java/se/llbit/chunky/main/Chunky.java +++ b/chunky/src/java/se/llbit/chunky/main/Chunky.java @@ -245,9 +245,6 @@ public static void main(final String[] args) { } ChunkyThread.interruptAndJoinAll(5, TimeUnit.SECONDS); - ForkJoinPool commonThreads = Chunky.getCommonThreads(); - commonThreads.shutdownNow(); // ForkJoinPool doesn't return any tasks that were awaiting execution (all canceled). - // We don't use the ForkJoinPool commonPool in chunky, but if we did there is no shutdown available so nothing changes. if (exitCode != 0) { System.exit(exitCode); @@ -350,7 +347,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; } @@ -361,7 +358,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(); } diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index 3284a7302d..ea3a544ff7 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -19,8 +19,11 @@ *
* *

Usage

- *

{@link Thread Threads} in chunky should extend this class, and {@link ExecutorService executor services} should - * be created with {@link #addExecutorService(Function)} to allow chunky to interrupt and join them before chunky closes.

+ *
    + *
  • {@link Thread Threads} in chunky should extend this class
    • + *
  • {@link ExecutorService Executor Services} should be created with {@link #addExecutorService(Function)}
    • + *
  • {@link ForkJoinPool Fork Join Pools} should be created with {@link #addForkJoinPool(ForkJoinPool)}
    • + *
* */ public class ChunkyThread extends Thread { @@ -55,6 +58,19 @@ public synchronized static E addExecutorService(Func return e; } + /** + * Add a {@link ForkJoinPool} to be interrupted and joined by chunky on shutdown + * + * @throws IllegalStateException When calling after {@link #interruptAndJoinAll(long, TimeUnit)} has been called. + */ + public synchronized static ForkJoinPool addForkJoinPool(ForkJoinPool pool) { + if (shutdownLatch.getCount() == 0) { + throw new IllegalStateException("Creating a fork join pool as chunky is stopping."); + } + executorServices.add(pool); + return pool; + } + /** * Await the joining of all threads managed by chunky. This method will wait indefinitely until a * shutdown happens to begin its timeout. From f5d4d139357ed04cbbe9c30284ac00754ab128ae Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Mon, 13 Jul 2026 08:57:10 +0100 Subject: [PATCH 06/17] Change ExecutorService thread factory --- .../src/java/se/llbit/util/concurrent/ChunkyThread.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index ea3a544ff7..e86a080cf8 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -53,7 +53,11 @@ public synchronized static E addExecutorService(Func if (shutdownLatch.getCount() == 0) { throw new IllegalStateException("Creating an executor service as chunky is stopping."); } - E e = executorServiceSupplier.apply(ChunkyThread::new); + E e = executorServiceSupplier.apply(r -> { // executor shutdown interrupts its own threads, so they don't need to be ChunkyThreads + Thread t = new Thread(r); + t.setDaemon(true); + return t; + }); executorServices.add(e); return e; } @@ -159,8 +163,6 @@ public static boolean interruptAndJoinAll(long timeout, @NotNull TimeUnit unit) synchronized (ChunkyThread.class) { shutdownLatch.countDown(); - // shut down executorServices BEFORE threads because they recreate their threads when they are interrupted and stop - // causing an infinite hang. for (ExecutorService executorService : executorServices) { executorService.shutdownNow(); } From 7163545b093ea0fdc33281d10b2353c70e26083f Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Mon, 13 Jul 2026 09:29:45 +0100 Subject: [PATCH 07/17] Add PluginApi to relevant ChunkyThread methods --- chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index e86a080cf8..07c9bd8b66 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -1,6 +1,7 @@ package se.llbit.util.concurrent; import se.llbit.chunky.main.Chunky; +import se.llbit.chunky.plugin.PluginApi; import se.llbit.util.annotation.NotNull; import java.util.ArrayList; @@ -36,6 +37,7 @@ public class ChunkyThread extends Thread { * * @throws IllegalStateException When calling after {@link #interruptAndJoinAll(long, TimeUnit)} has been called. */ + @PluginApi public synchronized static T addThread(T thread) { if (shutdownLatch.getCount() == 0) { throw new IllegalStateException("Creating a thread as chunky is stopping."); @@ -49,6 +51,7 @@ public synchronized static T addThread(T thread) { * * @throws IllegalStateException When calling after {@link #interruptAndJoinAll(long, TimeUnit)} has been called. */ + @PluginApi public synchronized static E addExecutorService(Function executorServiceSupplier) { if (shutdownLatch.getCount() == 0) { throw new IllegalStateException("Creating an executor service as chunky is stopping."); @@ -67,6 +70,7 @@ public synchronized static E addExecutorService(Func * * @throws IllegalStateException When calling after {@link #interruptAndJoinAll(long, TimeUnit)} has been called. */ + @PluginApi public synchronized static ForkJoinPool addForkJoinPool(ForkJoinPool pool) { if (shutdownLatch.getCount() == 0) { throw new IllegalStateException("Creating a fork join pool as chunky is stopping."); @@ -87,6 +91,7 @@ public synchronized static ForkJoinPool addForkJoinPool(ForkJoinPool pool) { * @param unit the time unit of the timeout argument * @return Whether all threads were joined before returning */ + @PluginApi public static boolean joinAll(long timeout, @NotNull TimeUnit unit) { /* * This method should not be synchronized because: From 297b535163413d0aabe671c4305d46a7dcacb22a Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Mon, 13 Jul 2026 09:32:28 +0100 Subject: [PATCH 08/17] Add shutdown hook to handle early System.exit() --- chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index 07c9bd8b66..47a1161ed1 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -32,6 +32,13 @@ public class ChunkyThread extends Thread { private static final Collection threads = new ArrayList<>(); private static final Collection executorServices = new ArrayList<>(); + static { + // If anyone calls System.exit() we still want to attempt to stop all threads + Runtime.getRuntime().addShutdownHook( + new Thread(() -> ChunkyThread.interruptAndJoinAll(0, TimeUnit.SECONDS)) // intentionally not ChunkyThread + ); + } + /** * Add a {@link Thread} to be interrupted and joined by chunky on shutdown * From 7f0c6615338a470031dd33e34d3a42e912f0def8 Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Mon, 13 Jul 2026 09:39:13 +0100 Subject: [PATCH 09/17] Put interruptAndJoinAll in a finally block --- .../src/java/se/llbit/chunky/main/Chunky.java | 50 ++++++++++--------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/chunky/src/java/se/llbit/chunky/main/Chunky.java b/chunky/src/java/se/llbit/chunky/main/Chunky.java index 14e0925915..0e09e96a18 100644 --- a/chunky/src/java/se/llbit/chunky/main/Chunky.java +++ b/chunky/src/java/se/llbit/chunky/main/Chunky.java @@ -217,35 +217,39 @@ public static void main(final String[] args) { if (cmdline.mode == CommandLineOptions.Mode.CLI_OPERATION) { exitCode = cmdline.exitCode; } else { - // Initialize the common thread pool. - getCommonThreads(); + try { + // Initialize the common thread pool. + getCommonThreads(); - Chunky chunky = new Chunky(cmdline.options); - chunky.headless = cmdline.mode == Mode.HEADLESS_RENDER || cmdline.mode == Mode.CREATE_SNAPSHOT; - chunky.loadPlugins(); + Chunky chunky = new Chunky(cmdline.options); + chunky.headless = cmdline.mode == Mode.HEADLESS_RENDER || cmdline.mode == Mode.CREATE_SNAPSHOT; + chunky.loadPlugins(); - try { - switch (cmdline.mode) { - case HEADLESS_RENDER: - exitCode = chunky.doHeadlessRender(); - break; - case CREATE_SNAPSHOT: - exitCode = chunky.doSnapshot(); - break; - case START_GUI: - ChunkyFx.startChunkyUI(chunky); - break; + try { + switch (cmdline.mode) { + case HEADLESS_RENDER: + exitCode = chunky.doHeadlessRender(); + break; + case CREATE_SNAPSHOT: + exitCode = chunky.doSnapshot(); + break; + case START_GUI: + ChunkyFx.startChunkyUI(chunky); + break; + } + } catch (Throwable t) { + // set receiver in case an exception was thrown before it was set in one of the start modes. + Log.setReceiver(ConsoleReceiver.INSTANCE, Level.INFO, Level.WARNING, Level.ERROR); + Log.error("Unchecked exception caused Chunky to close.", t); + exitCode = 2; + } + } finally { + if (!ChunkyThread.interruptAndJoinAll(5, TimeUnit.SECONDS)) { + Log.warn("Not all Chunky threads stopped before exiting."); } - } catch (Throwable t) { - // set receiver in case an exception was thrown before it was set in one of the start modes. - Log.setReceiver(ConsoleReceiver.INSTANCE, Level.INFO, Level.WARNING, Level.ERROR); - Log.error("Unchecked exception caused Chunky to close.", t); - exitCode = 2; } } - ChunkyThread.interruptAndJoinAll(5, TimeUnit.SECONDS); - if (exitCode != 0) { System.exit(exitCode); } From 22330c69b37437271a140b306f2fdee1421cdbea Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Fri, 24 Jul 2026 20:39:34 +0100 Subject: [PATCH 10/17] Switch back to ConsoleReceiver on UI stop --- chunky/src/java/se/llbit/chunky/ui/ChunkyFx.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/chunky/src/java/se/llbit/chunky/ui/ChunkyFx.java b/chunky/src/java/se/llbit/chunky/ui/ChunkyFx.java index 44a552b5f4..6226d4c834 100644 --- a/chunky/src/java/se/llbit/chunky/ui/ChunkyFx.java +++ b/chunky/src/java/se/llbit/chunky/ui/ChunkyFx.java @@ -29,6 +29,8 @@ import se.llbit.chunky.resources.SettingsDirectory; import se.llbit.chunky.ui.controller.ChunkyFxController; import se.llbit.fxutil.WindowPosition; +import se.llbit.log.ConsoleReceiver; +import se.llbit.log.Level; import se.llbit.log.Log; import java.io.File; @@ -79,6 +81,8 @@ public class ChunkyFx extends Application { @Override public void stop() throws Exception { + // UI is closing so we need to replace the receivers + Log.setReceiver(ConsoleReceiver.INSTANCE, Level.INFO, Level.WARNING, Level.ERROR); if(mainStage != null) { PersistentSettings.setWindowPosition(new WindowPosition(mainStage)); } From afb094700438aaf1e89eb8a4e2957377badf2a53 Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Fri, 24 Jul 2026 20:41:34 +0100 Subject: [PATCH 11/17] Move the shutdown hook out of ChunkyThread, now managed by Chunky itself --- .../src/java/se/llbit/chunky/main/Chunky.java | 69 ++++++++++--------- .../llbit/util/concurrent/ChunkyThread.java | 7 -- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/chunky/src/java/se/llbit/chunky/main/Chunky.java b/chunky/src/java/se/llbit/chunky/main/Chunky.java index 0e09e96a18..4b69de74d3 100644 --- a/chunky/src/java/se/llbit/chunky/main/Chunky.java +++ b/chunky/src/java/se/llbit/chunky/main/Chunky.java @@ -213,45 +213,52 @@ 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 { - try { - // Initialize the common thread pool. - getCommonThreads(); + // Initialize the common thread pool. + getCommonThreads(); - Chunky chunky = new Chunky(cmdline.options); - chunky.headless = cmdline.mode == Mode.HEADLESS_RENDER || cmdline.mode == Mode.CREATE_SNAPSHOT; - chunky.loadPlugins(); + Chunky chunky = new Chunky(cmdline.options); + chunky.headless = cmdline.mode == Mode.HEADLESS_RENDER || cmdline.mode == Mode.CREATE_SNAPSHOT; + chunky.loadPlugins(); - try { - switch (cmdline.mode) { - case HEADLESS_RENDER: - exitCode = chunky.doHeadlessRender(); - break; - case CREATE_SNAPSHOT: - exitCode = chunky.doSnapshot(); - break; - case START_GUI: - ChunkyFx.startChunkyUI(chunky); - break; - } - } catch (Throwable t) { - // set receiver in case an exception was thrown before it was set in one of the start modes. - Log.setReceiver(ConsoleReceiver.INSTANCE, Level.INFO, Level.WARNING, Level.ERROR); - Log.error("Unchecked exception caused Chunky to close.", t); - exitCode = 2; - } - } finally { - if (!ChunkyThread.interruptAndJoinAll(5, TimeUnit.SECONDS)) { - Log.warn("Not all Chunky threads stopped before exiting."); + 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: + exitCode = chunky.doHeadlessRender(); + break; + case CREATE_SNAPSHOT: + exitCode = chunky.doSnapshot(); + break; + case START_GUI: + ChunkyFx.startChunkyUI(chunky); + break; } + } catch (Throwable t) { + // set receiver in case an exception was thrown before it was set in one of the start modes. + Log.setReceiver(ConsoleReceiver.INSTANCE, Level.INFO, Level.WARNING, Level.ERROR); + Log.error("Unchecked exception caused Chunky to close.", t); + exitCode = 2; } + 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); } + } - if (exitCode != 0) { - System.exit(exitCode); + private void shutdown(int timeout, TimeUnit unit) { + if (!ChunkyThread.interruptAndJoinAll(timeout, unit)) { + Log.error("Not all threads were joined before shutting down."); // FIXME: list all alive threads? ThreadGroups are annoying. } } diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index 47a1161ed1..07c9bd8b66 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -32,13 +32,6 @@ public class ChunkyThread extends Thread { private static final Collection threads = new ArrayList<>(); private static final Collection executorServices = new ArrayList<>(); - static { - // If anyone calls System.exit() we still want to attempt to stop all threads - Runtime.getRuntime().addShutdownHook( - new Thread(() -> ChunkyThread.interruptAndJoinAll(0, TimeUnit.SECONDS)) // intentionally not ChunkyThread - ); - } - /** * Add a {@link Thread} to be interrupted and joined by chunky on shutdown * From f20b005e987f103f19d530a19085c048cf2b63b8 Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Fri, 24 Jul 2026 20:41:47 +0100 Subject: [PATCH 12/17] Remove PluginApi from joinAll --- chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java | 1 - 1 file changed, 1 deletion(-) diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index 07c9bd8b66..e7aeae8a9a 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -91,7 +91,6 @@ public synchronized static ForkJoinPool addForkJoinPool(ForkJoinPool pool) { * @param unit the time unit of the timeout argument * @return Whether all threads were joined before returning */ - @PluginApi public static boolean joinAll(long timeout, @NotNull TimeUnit unit) { /* * This method should not be synchronized because: From e69129b4fc27a09504edcf0136720300ecb39769 Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Fri, 24 Jul 2026 20:43:50 +0100 Subject: [PATCH 13/17] Refactor ChunkyThread joinAll to give stronger guarantees And hopefully be more readable --- .../llbit/util/concurrent/ChunkyThread.java | 84 +++++++++++-------- 1 file changed, 51 insertions(+), 33 deletions(-) diff --git a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java index e7aeae8a9a..51d4631f35 100644 --- a/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -89,6 +89,7 @@ public synchronized static ForkJoinPool addForkJoinPool(ForkJoinPool pool) { * * @param timeout The maximum time to wait AFTER a shutdown is initiated * @param unit the time unit of the timeout argument + * * @return Whether all threads were joined before returning */ public static boolean joinAll(long timeout, @NotNull TimeUnit unit) { @@ -102,8 +103,9 @@ public static boolean joinAll(long timeout, @NotNull TimeUnit unit) { while (true) { try { - // must wait for the latch as hitting the for loop below first causes immediate evaluation of the - // for loop iterator, potentially missing new threads. + // Must wait for the latch as hitting the for loop below first causes immediate evaluation of: + // - The for loop iterator, potentially missing new threads. + // - The end time, meaning waiting starts before shutdown begins. shutdownLatch.await(); break; } catch (InterruptedException e) { @@ -111,46 +113,25 @@ public static boolean joinAll(long timeout, @NotNull TimeUnit unit) { } } - long startTime = System.nanoTime(); - long endTime = startTime + unit.toNanos(timeout); - - boolean anyAlive = false; - + long endTime = System.nanoTime() + unit.toNanos(timeout); try { - for (ExecutorService executorService : executorServices) { - while (System.nanoTime() < endTime) { - try { - long waitTime = endTime - startTime; - if (waitTime > 0) { - executorService.awaitTermination(waitTime, TimeUnit.NANOSECONDS); - } - break; - } catch (InterruptedException e) { - interrupted = true; - } - } - anyAlive |= !executorService.isTerminated(); - } - for (Thread thread : ChunkyThread.threads) { - while (System.nanoTime() < endTime) { - try { - long waitTimeMillis = TimeUnit.NANOSECONDS.toMillis(endTime - startTime); - if (waitTimeMillis > 0) { - thread.join(waitTimeMillis); - } - break; - } catch (InterruptedException e) { - interrupted = true; + while (System.nanoTime() < endTime) { + try { + if (joinAllInterruptable(endTime)) { + // All threads are joined, skip the rest of the wait time. + return true; } + } catch (InterruptedException e) { + interrupted = true; } - anyAlive |= thread.isAlive(); } + // Got to the end of the wait time without joining everything, can give no guarantees + return false; } finally { if (interrupted) { Thread.currentThread().interrupt(); } } - return !anyAlive; } /** @@ -178,6 +159,43 @@ public static boolean interruptAndJoinAll(long timeout, @NotNull TimeUnit unit) return joinAll(timeout, unit); } + /** + * Await the joining of all threads managed by chunky. + * + *

This method is only safe to call if the {@link ChunkyThread#shutdownLatch} has been set.

+ * + *

WARNING: calling this from any thread registered with {@link #addThread(Thread)} may deadlock.

+ * + * @param endTimeNanos The time at which to stop waiting. + * + * @return Whether all threads were joined before returning + * + * @throws InterruptedException Propagates up when interrupted. The caller has no guarantee that shutdown has begun, + * or that any of the inner threads have been joined. + */ + private static boolean joinAllInterruptable(long endTimeNanos) throws InterruptedException { + // The intention here whether we return true or false, is to give the caller the most complete acquire load possible. + // Even if we reach the timeout given by the caller, we still establish a happens-before with every dead thread. + + boolean anyAlive = false; + for (ExecutorService executorService : executorServices) { + long waitTime = endTimeNanos - System.nanoTime(); + anyAlive |= !executorService.awaitTermination(waitTime, TimeUnit.NANOSECONDS); + } + for (Thread thread : threads) { + long waitTime = endTimeNanos - System.nanoTime(); + if (waitTime > 0) { + thread.join(waitTime); // joining with 0 is infinite wait time, very intuitive. + } + // Thread.isAlive() establishes a happens-before with the thread. As such the following are non-issues: + // - Not joining the thread, if waitTime <= 0 + // - The thread stopping between Thread.join() and Thread.isAlive(). + anyAlive |= thread.isAlive(); + } + + return !anyAlive; + } + private void setDefaults() { this.setDaemon(true); addThread(this); From f6acc0b5efdb086bfb82d914c7c665b99a3b45fe Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Thu, 23 Jul 2026 22:04:39 +0100 Subject: [PATCH 14/17] Remove jank in PluginManager --- .../java/se/llbit/chunky/plugin/loader/PluginManager.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java b/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java index b9a903a995..d17461fe34 100644 --- a/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java +++ b/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java @@ -72,18 +72,18 @@ public void load(Set pluginManifests, BiConsumer(pluginsToLoad).forEach(plugin -> { + for (Iterator 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()); } - }); + } } // report if any unloaded plugins remain (their dependencies never got loaded) From 58125374d2758f3d6268b8aac324d96b45cb812c Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Thu, 23 Jul 2026 22:10:18 +0100 Subject: [PATCH 15/17] Remove unnecessary chunky.plugins.maxLoadCycles jvm arg. The comment already explains that the worst case cannot take more cycles than the number of plugins to load, so we use that. --- .../chunky/plugin/loader/PluginManager.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java b/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java index d17461fe34..cfc15e3524 100644 --- a/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java +++ b/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java @@ -35,8 +35,6 @@ 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; public PluginManager(PluginLoader pluginLoader) { @@ -64,12 +62,13 @@ public void load(Set pluginManifests, BiConsumer 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 loadedPlugins = new HashSet<>(); int loadCycles = 0; - while (!pluginsToLoad.isEmpty() && loadCycles < MAX_CYCLES) { + while (!pluginsToLoad.isEmpty() && loadCycles < maxCycles) { Log.infof("Cycle %d", loadCycles); loadCycles++; for (Iterator iterator = pluginsToLoad.iterator(); iterator.hasNext(); ) { @@ -89,8 +88,9 @@ public void load(Set pluginManifests, BiConsumer Date: Fri, 24 Jul 2026 22:00:14 +0100 Subject: [PATCH 16/17] Implement plugin shutdown --- chunky/src/java/se/llbit/chunky/Plugin.java | 14 ++++ .../src/java/se/llbit/chunky/main/Chunky.java | 45 +++--------- .../chunky/plugin/loader/PluginManager.java | 69 ++++++++++++++++++- .../chunky/plugin/PluginManagerTest.java | 2 +- 4 files changed, 92 insertions(+), 38 deletions(-) diff --git a/chunky/src/java/se/llbit/chunky/Plugin.java b/chunky/src/java/se/llbit/chunky/Plugin.java index d8e244ee55..836e8c7624 100644 --- a/chunky/src/java/se/llbit/chunky/Plugin.java +++ b/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. + * + *

This function will be called after all {@link ChunkyThread}s have shut down. Chunky's UI thread may still + * be running.

+ *

This method will not be called if any {@link ChunkyThread} does not shut down within its given time limit

+ *

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 (SIGKILL & TerminateProcess), or an error within a native + * function.

+ */ + default void shutdown(Chunky chunky) { } } diff --git a/chunky/src/java/se/llbit/chunky/main/Chunky.java b/chunky/src/java/se/llbit/chunky/main/Chunky.java index 4b69de74d3..e444a0fa3c 100644 --- a/chunky/src/java/se/llbit/chunky/main/Chunky.java +++ b/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; @@ -87,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; @@ -108,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()); } @@ -257,7 +260,9 @@ public static void main(final String[] args) { } private void shutdown(int timeout, TimeUnit unit) { - if (!ChunkyThread.interruptAndJoinAll(timeout, 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. } } @@ -271,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 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) + ); } /** diff --git a/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java b/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java index cfc15e3524..6ccaf6eb27 100644 --- a/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java +++ b/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,17 +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 final PluginLoader pluginLoader; + private final LinkedList plugins = new LinkedList<>(); + private final AtomicBoolean isShutdown = new AtomicBoolean(false); public PluginManager(PluginLoader pluginLoader) { this.pluginLoader = pluginLoader; } - public void load(Set pluginManifests, BiConsumer onLoad) { + public void loadPluginsFromDirectory(File pluginsDirectory, BiConsumer 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 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 pluginManifests, BiConsumer 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. + *

Has no effect if already shutdown.

+ */ + public void shutdownPlugins(Consumer 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 pluginManifests, BiConsumer onLoad) { // create plugin objects Map> pluginsByName = new HashMap<>(); pluginManifests.forEach(manifest -> { @@ -80,7 +140,10 @@ public void load(Set pluginManifests, BiConsumer { + this.plugins.add(new PluginEntry(manifest, loadedPlugin)); + onLoad.accept(loadedPlugin, manifest); + }, plugin.getManifest()); } } } @@ -118,4 +181,6 @@ public static Optional parsePluginManifest(File pluginJar) { } return Optional.empty(); } + + private record PluginEntry(PluginManifest manifest, Plugin plugin) {} } diff --git a/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java b/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java index 8b6d85fd20..54e62e3126 100644 --- a/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java +++ b/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java @@ -164,7 +164,7 @@ private static void assertLoadOrder(Set manifests, Set e loadedPlugins.add(pluginManifest.name); }); - pluginLoader.load(manifests, (plugin, manifest) -> {}); + pluginLoader.loadPlugins(manifests, (plugin, manifest) -> {}); assertEquals(expectedPlugins, loadedPlugins); } } From 84906bc980a5ee851d3e0accbed21f33449e541d Mon Sep 17 00:00:00 2001 From: Tom Martin Date: Fri, 31 Jul 2026 16:43:37 +0100 Subject: [PATCH 17/17] Add support for inbuilt plugins --- .../src/java/se/llbit/chunky/main/Chunky.java | 8 +-- .../llbit/chunky/plugin/BuiltInPlugins.java | 31 ++++++++++ .../chunky/plugin/loader/JarPluginLoader.java | 9 ++- .../chunky/plugin/loader/PluginManager.java | 22 +++++-- .../chunky/plugin/loader/ResolvedPlugin.java | 4 +- .../plugin/manifest/PluginManifest.java | 42 +++++++------ .../chunky/plugin/PluginManagerTest.java | 62 +++++++++---------- 7 files changed, 111 insertions(+), 67 deletions(-) create mode 100644 chunky/src/java/se/llbit/chunky/plugin/BuiltInPlugins.java diff --git a/chunky/src/java/se/llbit/chunky/main/Chunky.java b/chunky/src/java/se/llbit/chunky/main/Chunky.java index e444a0fa3c..49b3a91987 100644 --- a/chunky/src/java/se/llbit/chunky/main/Chunky.java +++ b/chunky/src/java/se/llbit/chunky/main/Chunky.java @@ -18,7 +18,6 @@ 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; @@ -27,7 +26,6 @@ import se.llbit.chunky.plugin.*; import se.llbit.chunky.plugin.loader.PluginManager; import se.llbit.chunky.plugin.loader.JarPluginLoader; -import se.llbit.chunky.plugin.manifest.PluginManifest; import se.llbit.chunky.renderer.*; import se.llbit.chunky.renderer.RenderManager; import se.llbit.chunky.renderer.export.PictureExportFormat; @@ -40,10 +38,8 @@ import se.llbit.chunky.resources.ResourcePackLoader; import se.llbit.chunky.resources.SettingsDirectory; import se.llbit.chunky.ui.ChunkyFx; -import se.llbit.chunky.ui.controller.CreditsController; import se.llbit.chunky.ui.render.RenderControlsTabTransformer; import se.llbit.chunky.world.MaterialStore; -import se.llbit.json.JsonArray; import se.llbit.log.ConsoleReceiver; import se.llbit.log.Level; import se.llbit.log.Log; @@ -55,11 +51,9 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; -import java.nio.file.Path; import java.util.*; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; /** * Chunky is a Minecraft mapping and rendering tool created byJesper Öqvist (jesper@llbit.se). @@ -110,7 +104,7 @@ public static String getMainWindowTitle() { public Chunky(ChunkyOptions options) { this.options = options; - this.pluginManager = new PluginManager(new JarPluginLoader()); + this.pluginManager = new PluginManager(BuiltInPlugins.PLUGINS, new JarPluginLoader()); registerBlockProvider(new MinecraftBlockProvider()); registerBlockProvider(new LegacyMinecraftBlockProvider()); } diff --git a/chunky/src/java/se/llbit/chunky/plugin/BuiltInPlugins.java b/chunky/src/java/se/llbit/chunky/plugin/BuiltInPlugins.java new file mode 100644 index 0000000000..09ccb3e2f1 --- /dev/null +++ b/chunky/src/java/se/llbit/chunky/plugin/BuiltInPlugins.java @@ -0,0 +1,31 @@ +package se.llbit.chunky.plugin; + +import org.apache.maven.artifact.versioning.ArtifactVersion; +import org.apache.maven.artifact.versioning.DefaultArtifactVersion; +import org.apache.maven.artifact.versioning.VersionRange; +import se.llbit.chunky.main.Version; +import se.llbit.chunky.plugin.manifest.PluginManifest; + +import java.util.Collection; +import java.util.List; + +public class BuiltInPlugins { + /** + * The plugins built into chunky + */ + public static final Collection PLUGINS; + + /** + * Built-in plugins are the same version as chunky + */ + private static final ArtifactVersion BUILT_IN_PLUGINS_VERSION = new DefaultArtifactVersion(Version.getVersion()); + /** + * Built-in plugins target the current chunky version + */ + private static final VersionRange BUILT_IN_PLUGINS_VERSION_RANGE = VersionRange.createFromVersion(Version.getVersion()); + + static { + PLUGINS = List.of( + ); + } +} diff --git a/chunky/src/java/se/llbit/chunky/plugin/loader/JarPluginLoader.java b/chunky/src/java/se/llbit/chunky/plugin/loader/JarPluginLoader.java index 50d76e8a86..2a4c938d15 100644 --- a/chunky/src/java/se/llbit/chunky/plugin/loader/JarPluginLoader.java +++ b/chunky/src/java/se/llbit/chunky/plugin/loader/JarPluginLoader.java @@ -17,7 +17,14 @@ public class JarPluginLoader implements PluginLoader { public void load(BiConsumer onLoad, PluginManifest pluginManifest) { try { - Class pluginClass = loadPluginClass(pluginManifest.main, pluginManifest.pluginJar); + Class pluginClass; + if (pluginManifest.pluginJar.isPresent()) { + File pluginJar = pluginManifest.pluginJar.get(); + pluginClass = loadPluginClass(pluginManifest.mainClass, pluginJar); + } else { + // No plugin jar is present, class must already be loaded. + pluginClass = Class.forName(pluginManifest.mainClass); + } Plugin plugin = (Plugin) pluginClass.getDeclaredConstructor().newInstance(); onLoad.accept(plugin, pluginManifest); } catch (IOException | ClassNotFoundException e) { diff --git a/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java b/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java index 6ccaf6eb27..9467b1a9f7 100644 --- a/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java +++ b/chunky/src/java/se/llbit/chunky/plugin/loader/PluginManager.java @@ -43,7 +43,10 @@ public class PluginManager { private final LinkedList plugins = new LinkedList<>(); private final AtomicBoolean isShutdown = new AtomicBoolean(false); - public PluginManager(PluginLoader pluginLoader) { + private final Collection builtInPlugins; + + public PluginManager(Collection builtInPlugins, PluginLoader pluginLoader) { + this.builtInPlugins = builtInPlugins; this.pluginLoader = pluginLoader; } @@ -64,6 +67,7 @@ public void loadPluginsFromDirectory(File pluginsDirectory, BiConsumer pluginManifests, BiConsumer onLoad) { // Load plugins this.load(pluginManifests, (plugin, manifest) -> { - String jarName = manifest.pluginJar.getName(); - Log.infof("Loading plugin: %s", jarName); + String source = manifest.pluginJar.map(File::getName).orElse("Chunky Inbuilt Plugins"); + Log.infof("Loading plugin from: %s", source); try { onLoad.accept(plugin, manifest); } catch (Throwable t) { - Log.error("Plugin " + jarName + " failed to load.", t); + Log.error("Plugin " + source + " failed to load.", t); } Log.infof("Plugin loaded: %s %s", manifest.name, manifest.version); }); @@ -114,7 +118,13 @@ private void load(Set pluginManifests, BiConsumer { if (plugins.size() > 1) { // we report the error and hope it goes ok - Log.errorf("There are %d plugins with the name %s, this is extremely likely to break, proceeding anyway.", plugins.size(), name); + Log.errorf("There are %d plugins with the name %s, this is extremely likely to break, proceeding anyway. From: %s", + plugins.size(), + name, + plugins.stream().map(plugin -> + plugin.getManifest().getNameVersionString()).collect(Collectors.joining(", ") + ) + ); } }); @@ -137,7 +147,7 @@ private void load(Set pluginManifests, BiConsumer { diff --git a/chunky/src/java/se/llbit/chunky/plugin/loader/ResolvedPlugin.java b/chunky/src/java/se/llbit/chunky/plugin/loader/ResolvedPlugin.java index a0ef44272c..f679bab04e 100644 --- a/chunky/src/java/se/llbit/chunky/plugin/loader/ResolvedPlugin.java +++ b/chunky/src/java/se/llbit/chunky/plugin/loader/ResolvedPlugin.java @@ -19,7 +19,7 @@ public ResolvedPlugin(PluginManifest manifest) { */ public void resolveDependencies(Map> pluginsByName) { // Adds any plugin as a resolved dependency if it matches an unresolved dependency's name and version. - this.manifest.getDependencies().forEach(unresolvedDep -> { + this.manifest.dependencies.forEach(unresolvedDep -> { List resolvedDeps = pluginsByName.get(unresolvedDep.name); boolean resolved = false; for (ResolvedPlugin resolvedDep : resolvedDeps) { @@ -52,6 +52,6 @@ public Set getDependencies() { @Override public String toString() { - return this.manifest.name + ":" + this.manifest.version; + return this.manifest.getNameVersionString(); } } diff --git a/chunky/src/java/se/llbit/chunky/plugin/manifest/PluginManifest.java b/chunky/src/java/se/llbit/chunky/plugin/manifest/PluginManifest.java index 9b0a1cf582..99dae110d9 100644 --- a/chunky/src/java/se/llbit/chunky/plugin/manifest/PluginManifest.java +++ b/chunky/src/java/se/llbit/chunky/plugin/manifest/PluginManifest.java @@ -17,35 +17,37 @@ /** * An immutable class representing the manifest file (plugin.json) of a plugin. */ -public class PluginManifest { - public final File pluginJar; +public final class PluginManifest { + public final Optional pluginJar; + public final String mainClass; public final String name; public final String author; public final String description; public final ArtifactVersion version; public final VersionRange targetVersion; - public final String main; - private final Set dependencies; + public final Set dependencies; /** - * @param pluginJar Location of a valid plugin .jar file - * @param name The name of the plugin - * @param author The author of the plugin - * @param description The description of the plugin - * @param version The version of the plugin + * @param pluginJar Location of a valid plugin .jar file. If not present the plugin class must already be loaded. + * @param mainClass The main class of the plugin within its .jar + * @param name The name of the plugin + * @param author The author of the plugin + * @param description The description of the plugin + * @param version The version of the plugin * @param targetVersion The target chunky version range of the plugin - * @param main The main class of the plugin within its .jar - * @param dependencies The required dependencies of the plugin + * @param dependencies The required dependencies of the plugin */ - public PluginManifest(File pluginJar, String name, String author, String description, ArtifactVersion version, - VersionRange targetVersion, String main, Set dependencies) { + public PluginManifest(Optional pluginJar, String mainClass, + String name, String author, String description, + ArtifactVersion version, VersionRange targetVersion, + Set dependencies) { this.pluginJar = pluginJar; + this.mainClass = mainClass; this.name = name; this.author = author; this.description = description; this.version = version; this.targetVersion = targetVersion; - this.main = main; this.dependencies = dependencies; } @@ -106,15 +108,15 @@ public static Optional parse(JsonObject manifest, File pluginJar VersionRange targetVersionRange; try { - targetVersionRange = VersionRange.createFromVersionSpec(targetVersion); + targetVersionRange = VersionRange.createFromVersionSpec(targetVersion); } catch (InvalidVersionSpecificationException e) { - Log.error(String.format("Failed to parse plugin %s targetVersion %s into version range.", name, targetVersion), e); - targetVersionRange = VersionRange.createFromVersion(targetVersion); + Log.error(String.format("Failed to parse plugin %s targetVersion %s into version range.", name, targetVersion), e); + targetVersionRange = VersionRange.createFromVersion(targetVersion); } - return Optional.of(new PluginManifest(pluginJar, name, author, description, new DefaultArtifactVersion(version), targetVersionRange, main, dependencies)); + return Optional.of(new PluginManifest(Optional.of(pluginJar), main, name, author, description, new DefaultArtifactVersion(version), targetVersionRange, dependencies)); } - public Set getDependencies() { - return dependencies; + public String getNameVersionString() { + return this.name + ":" + this.version; } } diff --git a/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java b/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java index 54e62e3126..e7074a8f9e 100644 --- a/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java +++ b/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java @@ -25,15 +25,15 @@ public void testSimpleChainOfDependencies() throws InvalidVersionSpecificationEx // create 10 dependencies, each depending on the previous one HashSet manifests = new HashSet<>(); for (int i = 1; i < 10; i++) { - manifests.add(new PluginManifest(null, "test" + i, "author" + i, "desc" + i, - new DefaultArtifactVersion(i + ".0"), chunkyVersion, "test1.Main", + manifests.add(new PluginManifest(Optional.empty(), "test1.Main", "test" + i, "author" + i, "desc" + i, + new DefaultArtifactVersion(i + ".0"), chunkyVersion, Set.of( new PluginDependency("test" + (i + 1), VersionRange.createFromVersionSpec("[" + (i + 1) + ".0]")) ) )); } - manifests.add(new PluginManifest(null, "test10", "author10", "desc10", - new DefaultArtifactVersion("10.0"), chunkyVersion, "test10.Main", Collections.emptySet() + manifests.add(new PluginManifest(Optional.empty(), "test10.Main", "test10", "author10", "desc10", + new DefaultArtifactVersion("10.0"), chunkyVersion, Collections.emptySet() )); Set expectedLoadedPlugins = Set.of("test1", "test2", "test3", "test4", "test5", "test6", "test7", "test8", "test9", "test10"); @@ -51,16 +51,16 @@ public void testLoopOfDependencies() throws InvalidVersionSpecificationException // create 10 dependencies, each depending on the previous one HashSet manifests = new HashSet<>(); for (int i = 1; i < 10; i++) { - manifests.add(new PluginManifest(null, "test" + i, "author" + i, "desc" + i, - new DefaultArtifactVersion(i + ".0"), chunkyVersion, "test1.Main", + manifests.add(new PluginManifest(Optional.empty(), "test1.Main", "test" + i, "author" + i, "desc" + i, + new DefaultArtifactVersion(i + ".0"), chunkyVersion, Set.of( new PluginDependency("test" + (i + 1), VersionRange.createFromVersionSpec("[" + (i + 1) + ".0]")) ) )); } // The last one depends on the first. A loop! - manifests.add(new PluginManifest(null, "test10", "author10", "desc10", - new DefaultArtifactVersion("10.0"), chunkyVersion, "test10.Main", + manifests.add(new PluginManifest(Optional.empty(), "test10.Main", "test10", "author10", "desc10", + new DefaultArtifactVersion("10.0"), chunkyVersion, Set.of( new PluginDependency("test1", VersionRange.createFromVersionSpec("[1.0]")) ) @@ -82,63 +82,63 @@ public void testNormalCase() throws InvalidVersionSpecificationException { 3 > 2 > 10 <┘ */ Set manifests = new HashSet<>(); - manifests.add(new PluginManifest(null, "test10", "author10", "desc10", - new DefaultArtifactVersion("10.0"), chunkyVersion, "test10.Main", + manifests.add(new PluginManifest(Optional.empty(), "test10.Main", "test10", "author10", "desc10", + new DefaultArtifactVersion("10.0"), chunkyVersion, Collections.emptySet() )); - manifests.add(new PluginManifest(null, "test9", "author9", "desc9", - new DefaultArtifactVersion("9.0"), chunkyVersion, "test9.Main", + manifests.add(new PluginManifest(Optional.empty(), "test9.Main", "test9", "author9", "desc9", + new DefaultArtifactVersion("9.0"), chunkyVersion, Set.of( new PluginDependency("test8", VersionRange.createFromVersionSpec("[8.0]")), new PluginDependency("test7", VersionRange.createFromVersionSpec("[7.0]")) ) )); - manifests.add(new PluginManifest(null, "test8", "author8", "desc8", - new DefaultArtifactVersion("8.0"), chunkyVersion, "test8.Main", + manifests.add(new PluginManifest(Optional.empty(), "test8.Main", "test8", "author8", "desc8", + new DefaultArtifactVersion("8.0"), chunkyVersion, Set.of( new PluginDependency("test5", VersionRange.createFromVersionSpec("[5.0]")) ) )); - manifests.add(new PluginManifest(null, "test7", "author7", "desc7", - new DefaultArtifactVersion("7.0"), chunkyVersion, "test7.Main", + manifests.add(new PluginManifest(Optional.empty(), "test7.Main", "test7", "author7", "desc7", + new DefaultArtifactVersion("7.0"), chunkyVersion, Set.of( new PluginDependency("test4", VersionRange.createFromVersionSpec("[4.0]")), new PluginDependency("test2", VersionRange.createFromVersionSpec("[2.0]")) ) )); - manifests.add(new PluginManifest(null, "test6", "author6", "desc6", - new DefaultArtifactVersion("6.0"), chunkyVersion, "test6.Main", + manifests.add(new PluginManifest(Optional.empty(), "test6.Main", "test6", "author6", "desc6", + new DefaultArtifactVersion("6.0"), chunkyVersion, Set.of( new PluginDependency("test7", VersionRange.createFromVersionSpec("[7.0]")), new PluginDependency("test3", VersionRange.createFromVersionSpec("[3.0]")) ) )); - manifests.add(new PluginManifest(null, "test5", "author5", "desc5", - new DefaultArtifactVersion("5.0"), chunkyVersion, "test5.Main", + manifests.add(new PluginManifest(Optional.empty(), "test5.Main", "test5", "author5", "desc5", + new DefaultArtifactVersion("5.0"), chunkyVersion, Set.of( new PluginDependency("test4", VersionRange.createFromVersionSpec("[4.0]")) ) )); - manifests.add(new PluginManifest(null, "test4", "author4", "desc4", - new DefaultArtifactVersion("4.0"), chunkyVersion, "test4.Main", + manifests.add(new PluginManifest(Optional.empty(), "test4.Main", "test4", "author4", "desc4", + new DefaultArtifactVersion("4.0"), chunkyVersion, Set.of( new PluginDependency("test10", VersionRange.createFromVersionSpec("[10.0]")) ) )); - manifests.add(new PluginManifest(null, "test3", "author3", "desc3", - new DefaultArtifactVersion("3.0"), chunkyVersion, "test3.Main", + manifests.add(new PluginManifest(Optional.empty(), "test3.Main", "test3", "author3", "desc3", + new DefaultArtifactVersion("3.0"), chunkyVersion, Set.of( new PluginDependency("test2", VersionRange.createFromVersionSpec("[2.0]")) ) )); - manifests.add(new PluginManifest(null, "test2", "author2", "desc2", - new DefaultArtifactVersion("2.0"), chunkyVersion, "test2.Main", + manifests.add(new PluginManifest(Optional.empty(), "test2.Main", "test2", "author2", "desc2", + new DefaultArtifactVersion("2.0"), chunkyVersion, Set.of( new PluginDependency("test10", VersionRange.createFromVersionSpec("[10.0]")) ) )); - manifests.add(new PluginManifest(null, "test1", "author1", "desc1", - new DefaultArtifactVersion("1.0"), chunkyVersion, "test1.Main", + manifests.add(new PluginManifest(Optional.empty(), "test1.Main", "test1", "author1", "desc1", + new DefaultArtifactVersion("1.0"), chunkyVersion, Set.of( new PluginDependency("test9", VersionRange.createFromVersionSpec("[9.0]")), new PluginDependency("test6", VersionRange.createFromVersionSpec("[6.0]")) @@ -157,10 +157,10 @@ public void testNormalCase() throws InvalidVersionSpecificationException { */ private static void assertLoadOrder(Set manifests, Set expectedPlugins) { Set loadedPlugins = new HashSet<>(); - PluginManager pluginLoader = new PluginManager((onLoad, pluginManifest) -> { - System.out.println("Loaded " + pluginManifest.name + ":\n\tWith dependencies: " + String.join(", ", pluginManifest.getDependencies().stream().map(p -> p.name).toList())); + PluginManager pluginLoader = new PluginManager(Set.of(), (onLoad, pluginManifest) -> { + System.out.println("Loaded " + pluginManifest.name + ":\n\tWith dependencies: " + String.join(", ", pluginManifest.dependencies.stream().map(p -> p.name).toList())); - assertTrue(pluginManifest.getDependencies().stream().map(p -> p.name).allMatch(loadedPlugins::contains), () -> "Plugin " + pluginManifest.name + " does not have all dependencies loaded"); + assertTrue(pluginManifest.dependencies.stream().map(p -> p.name).allMatch(loadedPlugins::contains), () -> "Plugin " + pluginManifest.name + " does not have all dependencies loaded"); loadedPlugins.add(pluginManifest.name); });