diff --git a/chunky/src/java/se/llbit/chunky/Plugin.java b/chunky/src/java/se/llbit/chunky/Plugin.java index d8e244ee5..836e8c762 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 13a696535..49b3a9198 100644 --- a/chunky/src/java/se/llbit/chunky/main/Chunky.java +++ b/chunky/src/java/se/llbit/chunky/main/Chunky.java @@ -26,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; @@ -39,24 +38,22 @@ 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; import se.llbit.log.Receiver; import se.llbit.util.TaskTracker; +import se.llbit.util.concurrent.ChunkyThread; import java.io.File; 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.stream.Collectors; +import java.util.concurrent.TimeUnit; /** * Chunky is a Minecraft mapping and rendering tool created byJesper Öqvist (jesper@llbit.se). @@ -85,6 +82,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 +104,7 @@ public static String getMainWindowTitle() { public Chunky(ChunkyOptions options) { this.options = options; + this.pluginManager = new PluginManager(BuiltInPlugins.PLUGINS, new JarPluginLoader()); registerBlockProvider(new MinecraftBlockProvider()); registerBlockProvider(new LegacyMinecraftBlockProvider()); } @@ -211,9 +210,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 +220,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 +246,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 +270,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) + ); } /** @@ -342,7 +327,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 +338,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/chunky/plugin/BuiltInPlugins.java b/chunky/src/java/se/llbit/chunky/plugin/BuiltInPlugins.java new file mode 100644 index 000000000..09ccb3e2f --- /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 50d76e8a8..2a4c938d1 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 b9a903a99..9467b1a9f 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,19 +33,79 @@ 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 plugins = new LinkedList<>(); + private final AtomicBoolean isShutdown = new AtomicBoolean(false); + + private final Collection builtInPlugins; - public PluginManager(PluginLoader pluginLoader) { + public PluginManager(Collection builtInPlugins, PluginLoader pluginLoader) { + this.builtInPlugins = builtInPlugins; 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()); + pluginManifests.addAll(this.builtInPlugins); + + loadPlugins(pluginManifests, onLoad); + } + + public void loadPlugins(Set pluginManifests, BiConsumer onLoad) { + // Load plugins + this.load(pluginManifests, (plugin, manifest) -> { + 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 " + source + " 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 -> { @@ -56,7 +118,13 @@ public 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(", ") + ) + ); } }); @@ -64,33 +132,38 @@ 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++; - // new list to avoid CME due to removing inside iteration. - new ArrayList<>(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.getManifest().dependencies.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 +191,6 @@ public static Optional parsePluginManifest(File pluginJar) { } return Optional.empty(); } + + private record PluginEntry(PluginManifest manifest, Plugin plugin) {} } 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 a0ef44272..f679bab04 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 9b0a1cf58..99dae110d 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/java/se/llbit/chunky/renderer/DefaultRenderManager.java b/chunky/src/java/se/llbit/chunky/renderer/DefaultRenderManager.java index 9b091c2e4..6074c3577 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 51b129d5d..80cef5ea4 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 919a4f4c2..7d0878026 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 bc4bc6bf6..82d0f3f21 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 93f6532ac..6226d4c83 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,10 +81,11 @@ 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)); } - 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 dcac32703..251fdff9d 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 534926aea..5c507b056 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 8c61d5b59..58ef6be99 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 4b2500a90..d879dc157 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 01ab9ed1b..bdc35cd96 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 b25f0771c..841851a51 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 000000000..51d4631f3 --- /dev/null +++ b/chunky/src/java/se/llbit/util/concurrent/ChunkyThread.java @@ -0,0 +1,249 @@ +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; +import java.util.Collection; +import java.util.concurrent.*; +import java.util.function.Function; + +/** + * {@link Thread}/{@link ExecutorService} resource management. + *

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.
  • + *
  • Within the non-termination {@link Runtime} shutdown sequence give guarantees to shut down hooks about the state + * of chunky.
  • + *
+ * + *

Usage

+ *
    + *
  • {@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 { + private static final CountDownLatch shutdownLatch = new CountDownLatch(1); + 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(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."); + } + 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(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."); + } + 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; + } + + /** + * 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. + */ + @PluginApi + 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. + * + *

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 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. + * 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) { + 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. + // - The end time, meaning waiting starts before shutdown begins. + shutdownLatch.await(); + break; + } catch (InterruptedException e) { + interrupted = true; + } + } + + long endTime = System.nanoTime() + unit.toNanos(timeout); + try { + 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; + } + } + // Got to the end of the wait time without joining everything, can give no guarantees + return false; + } finally { + if (interrupted) { + Thread.currentThread().interrupt(); + } + } + } + + /** + * Interrupt and then await joining of all threads managed by chunky. + * + *

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 static boolean interruptAndJoinAll(long timeout, @NotNull TimeUnit unit) { + synchronized (ChunkyThread.class) { + shutdownLatch.countDown(); + + for (ExecutorService executorService : executorServices) { + executorService.shutdownNow(); + } + for (Thread thread : ChunkyThread.threads) { + thread.interrupt(); + } + } + + 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); + } + + /* 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(); + } +} diff --git a/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java b/chunky/src/test/se/llbit/chunky/plugin/PluginManagerTest.java index 8b6d85fd2..e7074a8f9 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,14 +157,14 @@ 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); }); - pluginLoader.load(manifests, (plugin, manifest) -> {}); + pluginLoader.loadPlugins(manifests, (plugin, manifest) -> {}); assertEquals(expectedPlugins, loadedPlugins); } }