diff --git a/src/main/java/net/minestom/server/MinecraftServer.java b/src/main/java/net/minestom/server/MinecraftServer.java index 36f2d33c8..11115976a 100644 --- a/src/main/java/net/minestom/server/MinecraftServer.java +++ b/src/main/java/net/minestom/server/MinecraftServer.java @@ -665,14 +665,13 @@ public final class MinecraftServer { if (stopping) return; stopping = true; LOGGER.info("Stopping Minestom server."); - extensionManager.unloadAllExtensions(); + LOGGER.info("Unloading all extensions."); + extensionManager.shutdown(); updateManager.stop(); schedulerManager.shutdown(); connectionManager.shutdown(); server.stop(); storageManager.getLoadedLocations().forEach(StorageLocation::close); - LOGGER.info("Unloading all extensions."); - extensionManager.shutdown(); LOGGER.info("Shutting down all thread pools."); benchmarkManager.disable(); MinestomTerminal.stop(); diff --git a/src/main/java/net/minestom/server/extensions/Extension.java b/src/main/java/net/minestom/server/extensions/Extension.java index e84a59d92..e0df3b2de 100644 --- a/src/main/java/net/minestom/server/extensions/Extension.java +++ b/src/main/java/net/minestom/server/extensions/Extension.java @@ -71,13 +71,6 @@ public abstract class Extension { } - /** - * Called after postTerminate when reloading an extension - */ - public void unload() { - - } - @NotNull public DiscoveredExtension getOrigin() { return origin; diff --git a/src/main/java/net/minestom/server/extensions/ExtensionManager.java b/src/main/java/net/minestom/server/extensions/ExtensionManager.java index 4c8b90960..32024fc68 100644 --- a/src/main/java/net/minestom/server/extensions/ExtensionManager.java +++ b/src/main/java/net/minestom/server/extensions/ExtensionManager.java @@ -72,6 +72,37 @@ public class ExtensionManager { this.loadOnStartup = loadOnStartup; } + @NotNull + public File getExtensionFolder() { + return extensionFolder; + } + + public @NotNull Path getExtensionDataRoot() { + return extensionDataRoot; + } + + public void setExtensionDataRoot(@NotNull Path dataRoot) { + this.extensionDataRoot = dataRoot; + } + + @NotNull + public Collection getExtensions() { + return immutableExtensions.values(); + } + + @Nullable + public Extension getExtension(@NotNull String name) { + return extensions.get(name.toLowerCase()); + } + + public boolean hasExtension(@NotNull String name) { + return extensions.containsKey(name); + } + + // + // Loading + // + /** * Loads all extensions in the extension folder into this server. *

@@ -187,6 +218,17 @@ public class ExtensionManager { } } + public boolean loadDynamicExtension(@NotNull File jarFile) throws FileNotFoundException { + if (!jarFile.exists()) { + throw new FileNotFoundException("File '" + jarFile.getAbsolutePath() + "' does not exists. Cannot load extension."); + } + + LOGGER.info("Discover dynamic extension from jar {}", jarFile.getAbsolutePath()); + DiscoveredExtension discoveredExtension = discoverFromJar(jarFile); + List extensionsToLoad = Collections.singletonList(discoveredExtension); + return loadExtensionList(extensionsToLoad); + } + /** * Loads an extension into Minestom. * @@ -583,135 +625,6 @@ public class ExtensionManager { } } - @NotNull - public File getExtensionFolder() { - return extensionFolder; - } - - public @NotNull Path getExtensionDataRoot() { - return extensionDataRoot; - } - - public void setExtensionDataRoot(@NotNull Path dataRoot) { - this.extensionDataRoot = dataRoot; - } - - @NotNull - public Collection getExtensions() { - return immutableExtensions.values(); - } - - @Nullable - public Extension getExtension(@NotNull String name) { - return extensions.get(name.toLowerCase()); - } - - public boolean hasExtension(@NotNull String name) { - return extensions.containsKey(name); - } - - private void unload(@NotNull Extension ext) { - ext.preTerminate(); - ext.terminate(); - // remove callbacks for this extension - String extensionName = ext.getOrigin().getName(); - ext.triggerChange(observer -> observer.onExtensionUnload(extensionName)); - // TODO: more callback types - - // Remove event node - EventNode eventNode = ext.getEventNode(); - MinecraftServer.getGlobalEventHandler().removeChild(eventNode); - - ext.postTerminate(); - ext.unload(); - - // remove as dependent of other extensions - // this avoids issues where a dependent extension fails to reload, and prevents the base extension to reload too - for (Extension e : extensions.values()) { - e.getDependents().remove(ext.getOrigin().getName()); - } - - String id = ext.getOrigin().getName().toLowerCase(); - // remove from loaded extensions - extensions.remove(id); - - // remove class loader, required to reload the classes - MinestomExtensionClassLoader classloader = ext.getOrigin().removeMinestomExtensionClassLoader(); - try { - // close resources - classloader.close(); - } catch (IOException e) { - MinecraftServer.getExceptionManager().handleException(e); - } - //TODO : Remove extension from dependents -// MinestomRootClassLoader.getInstance().removeChildInHierarchy(classloader); - } - - public boolean reload(@NotNull String extensionName) { - Extension ext = extensions.get(extensionName.toLowerCase()); - if (ext == null) { - throw new IllegalArgumentException("Extension " + extensionName + " is not currently loaded."); - } - - File originalJar = ext.getOrigin().getOriginalJar(); - if (originalJar == null) { - LOGGER.error("Cannot reload extension {} that is not from a .jar file!", extensionName); - return false; - } - - LOGGER.info("Reload extension {} from jar file {}", extensionName, originalJar.getAbsolutePath()); - List dependents = new LinkedList<>(ext.getDependents()); // copy dependents list - List originalJarsOfDependents = new LinkedList<>(); - - for (String dependentID : dependents) { - Extension dependentExt = extensions.get(dependentID.toLowerCase()); - File dependentOriginalJar = dependentExt.getOrigin().getOriginalJar(); - originalJarsOfDependents.add(dependentOriginalJar); - if (dependentOriginalJar == null) { - LOGGER.error("Cannot reload extension {} that is not from a .jar file!", dependentID); - return false; - } - - LOGGER.info("Unloading dependent extension {} (because it depends on {})", dependentID, extensionName); - unload(dependentExt); - } - - LOGGER.info("Unloading extension {}", extensionName); - unload(ext); - - System.gc(); - - // ext and its dependents should no longer be referenced from now on - - // rediscover extension to reload. We allow dependency changes, so we need to fully reload it - List extensionsToReload = new LinkedList<>(); - LOGGER.info("Rediscover extension {} from jar {}", extensionName, originalJar.getAbsolutePath()); - DiscoveredExtension rediscoveredExtension = discoverFromJar(originalJar); - extensionsToReload.add(rediscoveredExtension); - - for (File dependentJar : originalJarsOfDependents) { - // rediscover dependent extension to reload - LOGGER.info("Rediscover dependent extension (depends on {}) from jar {}", extensionName, dependentJar.getAbsolutePath()); - extensionsToReload.add(discoverFromJar(dependentJar)); - } - - // ensure correct order of dependencies - loadExtensionList(extensionsToReload); - - return true; - } - - public boolean loadDynamicExtension(@NotNull File jarFile) throws FileNotFoundException { - if (!jarFile.exists()) { - throw new FileNotFoundException("File '" + jarFile.getAbsolutePath() + "' does not exists. Cannot load extension."); - } - - LOGGER.info("Discover dynamic extension from jar {}", jarFile.getAbsolutePath()); - DiscoveredExtension discoveredExtension = discoverFromJar(jarFile); - List extensionsToLoad = Collections.singletonList(discoveredExtension); - return loadExtensionList(extensionsToLoad); - } - private boolean loadExtensionList(@NotNull List extensionsToLoad) { // ensure correct order of dependencies LOGGER.debug("Reorder extensions to ensure proper load order"); @@ -747,7 +660,23 @@ public class ExtensionManager { return true; } - public void unloadExtension(@NotNull String extensionName) { + // + // Shutdown / Unload + // + + /** + * Shutdowns all the extensions by unloading them. + */ + public void shutdown() {// copy names, as the extensions map will be modified via the calls to unload + Set extensionNames = new HashSet<>(extensions.keySet()); + for (String ext : extensionNames) { + if (extensions.containsKey(ext)) { // is still loaded? Because extensions can depend on one another, it might have already been unloaded + unloadExtension(ext); + } + } + } + + private void unloadExtension(@NotNull String extensionName) { Extension ext = extensions.get(extensionName.toLowerCase()); if (ext == null) { @@ -764,31 +693,32 @@ public class ExtensionManager { LOGGER.info("Unloading extension {}", extensionName); unload(ext); - - // call GC to try to get rid of classes and classloader - System.gc(); } - /** - * Shutdowns all the extensions by unloading them. - */ - public void shutdown() { - //todo(mattw) what is different here from the method below? - for (Extension extension : getExtensions()) { - extension.unload(); - } - } + private void unload(@NotNull Extension ext) { + ext.preTerminate(); + ext.terminate(); + // remove callbacks for this extension + String extensionName = ext.getOrigin().getName(); + ext.triggerChange(observer -> observer.onExtensionUnload(extensionName)); - /** - * Unloads all extensions - */ - public void unloadAllExtensions() { - // copy names, as the extensions map will be modified via the calls to unload - Set extensionNames = new HashSet<>(extensions.keySet()); - for (String ext : extensionNames) { - if (extensions.containsKey(ext)) { // is still loaded? Because extensions can depend on one another, it might have already been unloaded - unloadExtension(ext); - } + // Remove event node + EventNode eventNode = ext.getEventNode(); + MinecraftServer.getGlobalEventHandler().removeChild(eventNode); + + ext.postTerminate(); + + // remove from loaded extensions + String id = ext.getOrigin().getName().toLowerCase(); + extensions.remove(id); + + // cleanup classloader + try { + MinestomExtensionClassLoader classloader = ext.getOrigin().removeMinestomExtensionClassLoader(); + classloader.close(); + //todo child classloaders, also since there is no reload, is this actually necessary? + } catch (IOException e) { + MinecraftServer.getExceptionManager().handleException(e); } } } diff --git a/src/test/java/demo/commands/ReloadExtensionCommand.java b/src/test/java/demo/commands/ReloadExtensionCommand.java deleted file mode 100644 index cf710b16d..000000000 --- a/src/test/java/demo/commands/ReloadExtensionCommand.java +++ /dev/null @@ -1,79 +0,0 @@ -package demo.commands; - -import net.kyori.adventure.text.Component; -import net.minestom.server.MinecraftServer; -import net.minestom.server.command.CommandSender; -import net.minestom.server.command.builder.Command; -import net.minestom.server.command.builder.CommandContext; -import net.minestom.server.command.builder.arguments.ArgumentString; -import net.minestom.server.command.builder.arguments.ArgumentType; -import net.minestom.server.command.builder.exception.ArgumentSyntaxException; -import net.minestom.server.extensions.Extension; -import net.minestom.server.extensions.ExtensionManager; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.PrintStream; -import java.nio.charset.StandardCharsets; - -public class ReloadExtensionCommand extends Command { - - // the extensions name as an array - private static String[] extensionsName; - - static { - ReloadExtensionCommand.extensionsName = MinecraftServer.getExtensionManager().getExtensions() - .stream() - .map(extension -> extension.getOrigin().getName()) - .toArray(String[]::new); - } - - private final ArgumentString extensionName; - - public ReloadExtensionCommand() { - super("reload"); - - setDefaultExecutor(this::usage); - - extensionName = ArgumentType.String("extensionName"); - - setArgumentCallback(this::gameModeCallback, extensionName); - - addSyntax(this::execute, extensionName); - } - - private void usage(CommandSender sender, CommandContext context) { - sender.sendMessage(Component.text("Usage: /reload ")); - } - - private void execute(CommandSender sender, CommandContext context) { - final String name = context.get(extensionName); - sender.sendMessage(Component.text("extensionName = " + name + "....")); - - ExtensionManager extensionManager = MinecraftServer.getExtensionManager(); - Extension ext = extensionManager.getExtension(name); - if (ext != null) { - try { - extensionManager.reload(name); - } catch (Throwable t) { - try { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - t.printStackTrace(); - t.printStackTrace(new PrintStream(baos)); - baos.flush(); - baos.close(); - String contents = new String(baos.toByteArray(), StandardCharsets.UTF_8); - contents.lines().map(Component::text).forEach(sender::sendMessage); - } catch (IOException e) { - e.printStackTrace(); - } - } - } else { - sender.sendMessage(Component.text("Extension '" + name + "' does not exist.")); - } - } - - private void gameModeCallback(CommandSender sender, ArgumentSyntaxException argumentSyntaxException) { - sender.sendMessage(Component.text("'" + argumentSyntaxException.getInput() + "' is not a valid extension name!")); - } -} diff --git a/src/test/java/demo/commands/UnloadExtensionCommand.java b/src/test/java/demo/commands/UnloadExtensionCommand.java deleted file mode 100644 index 6bf6a8bc6..000000000 --- a/src/test/java/demo/commands/UnloadExtensionCommand.java +++ /dev/null @@ -1,71 +0,0 @@ -package demo.commands; - -import net.kyori.adventure.text.Component; -import net.minestom.server.MinecraftServer; -import net.minestom.server.command.CommandSender; -import net.minestom.server.command.builder.Command; -import net.minestom.server.command.builder.CommandContext; -import net.minestom.server.command.builder.arguments.Argument; -import net.minestom.server.command.builder.arguments.ArgumentType; -import net.minestom.server.command.builder.exception.ArgumentSyntaxException; -import net.minestom.server.extensions.Extension; -import net.minestom.server.extensions.ExtensionManager; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.PrintStream; -import java.nio.charset.StandardCharsets; - -public class UnloadExtensionCommand extends Command { - - private final Argument extensionName; - - public UnloadExtensionCommand() { - super("unload"); - - setDefaultExecutor(this::usage); - - extensionName = ArgumentType.String("extensionName").map((input) -> { - Extension extension = MinecraftServer.getExtensionManager().getExtension(input); - - if (extension == null) throw new ArgumentSyntaxException("The specified extension was not found", input, 1); - - return extension; - }); - - setArgumentCallback(this::extensionCallback, extensionName); - - addSyntax(this::execute, extensionName); - } - - private void usage(CommandSender sender, CommandContext context) { - sender.sendMessage(Component.text("Usage: /unload ")); - } - - private void execute(CommandSender sender, CommandContext context) { - final Extension ext = context.get(extensionName); - sender.sendMessage(Component.text("extensionName = " + ext.getOrigin().getName() + "....")); - - ExtensionManager extensionManager = MinecraftServer.getExtensionManager(); - - try { - extensionManager.unloadExtension(ext.getOrigin().getName()); - } catch (Throwable t) { - try { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - t.printStackTrace(); - t.printStackTrace(new PrintStream(baos)); - baos.flush(); - baos.close(); - String contents = baos.toString(StandardCharsets.UTF_8); - contents.lines().map(Component::text).forEach(sender::sendMessage); - } catch (IOException e) { - e.printStackTrace(); - } - } - } - - private void extensionCallback(CommandSender sender, ArgumentSyntaxException exception) { - sender.sendMessage(Component.text("'" + exception.getInput() + "' is not a valid extension name!")); - } -} diff --git a/src/test/java/improveextensions/unloadcallbacks/UnloadCallbacksExtension.java b/src/test/java/improveextensions/unloadcallbacks/UnloadCallbacksExtension.java deleted file mode 100644 index 9ee5e211e..000000000 --- a/src/test/java/improveextensions/unloadcallbacks/UnloadCallbacksExtension.java +++ /dev/null @@ -1,125 +0,0 @@ -package improveextensions.unloadcallbacks; - -import net.minestom.server.MinecraftServer; -import net.minestom.server.coordinate.Vec; -import net.minestom.server.entity.EntityCreature; -import net.minestom.server.entity.EntityType; -import net.minestom.server.event.GlobalEventHandler; -import net.minestom.server.event.entity.EntityTickEvent; -import net.minestom.server.event.instance.InstanceTickEvent; -import net.minestom.server.extensions.Extension; -import net.minestom.server.extensions.isolation.MinestomExtensionClassLoader; -import net.minestom.server.instance.Instance; -import net.minestom.server.utils.time.TimeUnit; -import org.junit.jupiter.api.Assertions; -import org.opentest4j.AssertionFailedError; - -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Consumer; - -public class UnloadCallbacksExtension extends Extension { - - private boolean ticked1 = false; - private boolean ticked2 = false; - private boolean tickedScheduledNonTransient = false; - private boolean tickedScheduledTransient = false; - private boolean zombieTicked = false; - private boolean instanceTicked = false; - private final Consumer callback = this::onTick; - - private void onTick(InstanceTickEvent e) { - ticked1 = true; - } - - @Override - public void initialize() { - GlobalEventHandler globalEvents = MinecraftServer.getGlobalEventHandler(); - // this callback will be automatically removed when unloading the extension - globalEvents.addListener(InstanceTickEvent.class, callback); - // this one too - globalEvents.addListener(InstanceTickEvent.class, e -> ticked2 = true); - - Instance instance = MinecraftServer.getInstanceManager().getInstances().stream().findFirst().orElseThrow(); - - // add an event callback on an instance - globalEvents.addListener(InstanceTickEvent.class, e -> instanceTicked = true); - instance.loadChunk(0, 0); - - // add an event callback on an entity - EntityCreature zombie = new EntityCreature(EntityType.ZOMBIE); - globalEvents.addListener(EntityTickEvent.class, entityTickEvent -> { - if (entityTickEvent.getEntity() == zombie) { - zombieTicked = true; - } - }); - zombie.setInstance(instance, new Vec(8, 64, 8) /* middle of chunk */); - - // this callback will be cancelled - MinecraftServer.getSchedulerManager().buildTask(() -> { - tickedScheduledNonTransient = true; - }).repeat(100L, TimeUnit.MILLISECOND).schedule(); - - // this callback will NOT be cancelled - MinecraftServer.getSchedulerManager().buildTask(() -> { - tickedScheduledTransient = true; - }).repeat(100L, TimeUnit.MILLISECOND).schedule(); - - try { - Assertions.assertNotNull(MinestomExtensionClassLoader.findExtensionObjectOwner(callback)); - Assertions.assertEquals("UnloadCallbacksExtension", MinestomExtensionClassLoader.findExtensionObjectOwner(callback)); - } catch (AssertionFailedError e) { - e.printStackTrace(); - System.exit(-1); - } - - MinecraftServer.getSchedulerManager().buildTask(() -> { - // unload self - MinecraftServer.getExtensionManager().unloadExtension(getOrigin().getName()); - }).delay(1L, TimeUnit.SECOND).schedule(); - } - - @Override - public void terminate() { - new Thread(() -> { - try { - // wait for complete termination of this extension - Thread.sleep(10); - ticked1 = false; - ticked2 = false; - tickedScheduledNonTransient = false; - tickedScheduledTransient = false; - instanceTicked = false; - zombieTicked = false; - } catch (InterruptedException e) { - e.printStackTrace(); - } - }).start(); - - AtomicBoolean executedDelayTaskAfterTerminate = new AtomicBoolean(false); - // because terminate is called just before unscheduling and removing event callbacks, - // the following task will never be executed, because it is not transient - MinecraftServer.getSchedulerManager().buildTask(() -> { - executedDelayTaskAfterTerminate.set(true); - }).delay(100L, TimeUnit.MILLISECOND).schedule(); - - // this shutdown tasks will not be executed because it is not transient - MinecraftServer.getSchedulerManager().buildShutdownTask(() -> Assertions.fail("This shutdown task should be unloaded when the extension is")); - - MinecraftServer.getSchedulerManager().buildTask(() -> { - // Make sure callbacks are disabled - try { - Assertions.assertFalse(ticked1, "ticked1 should be false because the callback has been unloaded"); - Assertions.assertFalse(ticked2, "ticked2 should be false because the callback has been unloaded"); - Assertions.assertFalse(tickedScheduledNonTransient, "tickedScheduledNonTransient should be false because the callback has been unloaded"); - Assertions.assertFalse(zombieTicked, "zombieTicked should be false because the callback has been unloaded"); - Assertions.assertFalse(instanceTicked, "instanceTicked should be false because the callback has been unloaded"); - Assertions.assertTrue(tickedScheduledTransient, "tickedScheduledNonTransient should be true because the callback has NOT been unloaded"); - Assertions.assertFalse(executedDelayTaskAfterTerminate.get(), "executedDelayTaskAfterTerminate should be false because the callback has been unloaded before executing"); - System.out.println("All tests passed."); - } catch (AssertionFailedError e) { - e.printStackTrace(); - } - MinecraftServer.stopCleanly(); // TODO: fix deadlock which happens because stopCleanly waits on completion of scheduler tasks - }).delay(1L, TimeUnit.SECOND).schedule(); - } -} diff --git a/src/test/java/improveextensions/unloadextensiononstop/UnloadExtensionOnStop.java b/src/test/java/improveextensions/unloadextensiononstop/UnloadExtensionOnStop.java deleted file mode 100644 index 08694dd08..000000000 --- a/src/test/java/improveextensions/unloadextensiononstop/UnloadExtensionOnStop.java +++ /dev/null @@ -1,26 +0,0 @@ -package improveextensions.unloadextensiononstop; - -import net.minestom.server.MinecraftServer; -import net.minestom.server.extensions.Extension; -import net.minestom.server.utils.time.TimeUnit; -import org.junit.jupiter.api.Assertions; - -public class UnloadExtensionOnStop extends Extension { - - private boolean terminated = false; - - @Override - public void initialize() { - MinecraftServer.getSchedulerManager().buildShutdownTask(() -> { - Assertions.assertTrue(terminated, "Extension should have been terminated on shutdown."); - System.out.println("All tests passed."); - }); - - MinecraftServer.getSchedulerManager().buildTask(MinecraftServer::stopCleanly).delay(1L, TimeUnit.SECOND).schedule(); - } - - @Override - public void terminate() { - terminated = true; - } -}