diff --git a/src/main/java/net/minestom/server/extensions/DiscoveredExtension.java b/src/main/java/net/minestom/server/extensions/DiscoveredExtension.java index 8e1f13ae2..c20722a9d 100644 --- a/src/main/java/net/minestom/server/extensions/DiscoveredExtension.java +++ b/src/main/java/net/minestom/server/extensions/DiscoveredExtension.java @@ -1,7 +1,6 @@ package net.minestom.server.extensions; import com.google.gson.JsonObject; -import net.minestom.server.MinecraftServer; import net.minestom.server.extensions.isolation.MinestomExtensionClassLoader; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -11,7 +10,6 @@ import org.slf4j.LoggerFactory; import java.io.File; import java.net.URL; import java.nio.file.Path; -import java.util.Arrays; import java.util.LinkedList; import java.util.List; @@ -21,8 +19,6 @@ import java.util.List; * This has no constructor as its properties are set via GSON. */ public final class DiscoveredExtension { - private static final ExtensionManager EXTENSION_MANAGER = MinecraftServer.getExtensionManager(); - /** Static logger for this class. */ public static final Logger LOGGER = LoggerFactory.getLogger(DiscoveredExtension.class); @@ -68,7 +64,7 @@ public final class DiscoveredExtension { transient private Path dataDirectory; /** The class loader that powers it. */ - transient private MinestomExtensionClassLoader minestomExtensionClassLoader; + transient private MinestomExtensionClassLoader classLoader; @NotNull public String getName() { @@ -117,19 +113,15 @@ public final class DiscoveredExtension { this.dataDirectory = dataDirectory; } - MinestomExtensionClassLoader removeMinestomExtensionClassLoader() { - MinestomExtensionClassLoader oldClassLoader = getMinestomExtensionClassLoader(); - setMinestomExtensionClassLoader(null); - return oldClassLoader; + public void createClassLoader() { + if (classLoader != null) throw new IllegalStateException("Extension classloader has already been created!"); + final URL[] urls = this.files.toArray(new URL[0]); + classLoader = new MinestomExtensionClassLoader(this.getName(), urls); } - void setMinestomExtensionClassLoader(@Nullable MinestomExtensionClassLoader minestomExtensionClassLoader) { - this.minestomExtensionClassLoader = minestomExtensionClassLoader; - } - - @Nullable - public MinestomExtensionClassLoader getMinestomExtensionClassLoader() { - return this.minestomExtensionClassLoader; + @NotNull + public MinestomExtensionClassLoader getClassLoader() { + return classLoader; } /** @@ -206,50 +198,6 @@ public final class DiscoveredExtension { return meta; } - public MinestomExtensionClassLoader makeClassLoader() { - final URL[] urls = this.files.toArray(new URL[0]); - - MinestomExtensionClassLoader loader = new MinestomExtensionClassLoader(this.getName(), urls); - - System.out.println("CREATED " + loader + " WITH " + Arrays.toString(urls)); - - // add children to the dependencies -// for (String dependencyName : this.getDependencies()) { -// DiscoveredExtension dependency = discoveredExtensions.stream() -// .filter(ext -> ext.getName().equalsIgnoreCase(dependencyName)) -// .findFirst().orElse(null); -// -// if (dependency != null) { -// MinestomExtensionClassLoader dependencyLoader = dependency.getMinestomExtensionClassLoader(); -// assert dependencyLoader != null; //TODO: Better error handling -// -// loader.addChild(dependencyLoader); -// } else { -// //TODO: Better error handling -// throw new RuntimeException("Missing dependency " + dependencyName); -// } -// } - -// if (this.getDependencies().length == 0 || MinecraftServer.getExtensionManager() == null) { // it also may invoked in early class loader -// // orphaned extension, we can insert it directly -// root.addChild(loader); -// } else { -// // add children to the dependencies -// for (String dependency : this.getDependencies()) { -// if (MinecraftServer.getExtensionManager().hasExtension(dependency.toLowerCase())) { -// MinestomExtensionClassLoader parentLoader = MinecraftServer.getExtensionManager().getExtension(dependency.toLowerCase()).getOrigin().getMinestomExtensionClassLoader(); -// -// // TODO should never happen but replace with better throws error. -// assert parentLoader != null; -// -// parentLoader.addChild(loader); -// } -// } -// } - - return loader; - } - /** * The status this extension has, all are breakpoints. * diff --git a/src/main/java/net/minestom/server/extensions/Extension.java b/src/main/java/net/minestom/server/extensions/Extension.java index e0df3b2de..70723dca2 100644 --- a/src/main/java/net/minestom/server/extensions/Extension.java +++ b/src/main/java/net/minestom/server/extensions/Extension.java @@ -145,7 +145,7 @@ public abstract class Extension { */ public @Nullable InputStream getPackagedResource(@NotNull String fileName) { try { - final URL url = getOrigin().getMinestomExtensionClassLoader().getResource(fileName); + final URL url = getOrigin().getClassLoader().getResource(fileName); if (url == null) { getLogger().debug("Resource not found: {}", fileName); return null; diff --git a/src/main/java/net/minestom/server/extensions/ExtensionManager.java b/src/main/java/net/minestom/server/extensions/ExtensionManager.java index 794190365..40a312bc3 100644 --- a/src/main/java/net/minestom/server/extensions/ExtensionManager.java +++ b/src/main/java/net/minestom/server/extensions/ExtensionManager.java @@ -169,6 +169,7 @@ public class ExtensionManager { } //TODO(mattw) What is the purpose of this? + // Periodically cleanup observers MinecraftServer.getSchedulerManager().buildTask(() -> { for (Extension ext : extensions.values()) { @@ -189,9 +190,7 @@ public class ExtensionManager { while (extensionIterator.hasNext()) { DiscoveredExtension discoveredExtension = extensionIterator.next(); try { - MinestomExtensionClassLoader classLoader = discoveredExtension.makeClassLoader(); - discoveredExtension.setMinestomExtensionClassLoader(classLoader); - System.out.println("SET " + discoveredExtension.getName() + " TO " + discoveredExtension.getMinestomExtensionClassLoader()); + discoveredExtension.createClassLoader(); } catch (Exception e) { discoveredExtension.loadStatus = DiscoveredExtension.LoadStatus.FAILED_TO_SETUP_CLASSLOADER; MinecraftServer.getExceptionManager().handleException(e); @@ -243,7 +242,7 @@ public class ExtensionManager { String extensionName = discoveredExtension.getName(); String mainClass = discoveredExtension.getEntrypoint(); - MinestomExtensionClassLoader loader = discoveredExtension.getMinestomExtensionClassLoader(); + MinestomExtensionClassLoader loader = discoveredExtension.getClassLoader(); if (extensions.containsKey(extensionName.toLowerCase())) { LOGGER.error("An extension called '{}' has already been registered.", extensionName); @@ -589,17 +588,17 @@ public class ExtensionManager { LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved); } - MinestomExtensionClassLoader extensionClassLoader = discoveredExtension.getMinestomExtensionClassLoader(); + MinestomExtensionClassLoader extensionClassLoader = discoveredExtension.getClassLoader(); for (String dependencyName : discoveredExtension.getDependencies()) { var resolved = extensions.stream() .filter(ext -> ext.getName().equalsIgnoreCase(dependencyName)) .findFirst().orElse(null); - - MinestomExtensionClassLoader dependencyClassLoader = resolved.getMinestomExtensionClassLoader(); - if (dependencyClassLoader == null) { - throw new IllegalStateException("Dependency '" + dependencyName + "' of '" + discoveredExtension.getName() + "' has not been loaded."); + if (resolved == null) { + throw new IllegalStateException("Unknown dependency '" + dependencyName + "' of '" + discoveredExtension.getName() + "'"); } + MinestomExtensionClassLoader dependencyClassLoader = resolved.getClassLoader(); + extensionClassLoader.addChild(dependencyClassLoader); LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved); } @@ -613,10 +612,9 @@ public class ExtensionManager { } private void addDependencyFile(@NotNull ResolvedDependency dependency, @NotNull DiscoveredExtension extension) { - System.out.println("ADDING DEPENDENCY " + dependency.getName() + " TO " + extension.getName()); URL location = dependency.getContentsLocation(); extension.files.add(location); - extension.getMinestomExtensionClassLoader().addURL(location); + extension.getClassLoader().addURL(location); LOGGER.trace("Added dependency {} to extension {} classpath", location.toExternalForm(), extension.getName()); // recurse to add full dependency tree @@ -717,12 +715,6 @@ public class ExtensionManager { 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); - } + // TODO: Is it necessary to remove the CLs since this is only called on shutdown? } } diff --git a/src/main/java/net/minestom/server/extensions/isolation/HierarchyClassLoader.java b/src/main/java/net/minestom/server/extensions/isolation/HierarchyClassLoader.java index 5477c9bf6..3f7349e97 100644 --- a/src/main/java/net/minestom/server/extensions/isolation/HierarchyClassLoader.java +++ b/src/main/java/net/minestom/server/extensions/isolation/HierarchyClassLoader.java @@ -30,16 +30,12 @@ public abstract class HierarchyClassLoader extends URLClassLoader { @Override protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { try { - System.out.println("TRYING 2 LOAD " + name + " IN " + getName()); return super.loadClass(name, resolve); } catch (ClassNotFoundException e) { - System.out.println("COULD NOT LOAD 2, TRYING CHILDREN"); for (MinestomExtensionClassLoader child : children) { try { return child.loadClass(name, resolve); - } catch (ClassNotFoundException ignored) { - System.out.println("NOT FOUND IN " + child.getName() + " EITHER"); - } + } catch (ClassNotFoundException ignored) {} } throw e; } diff --git a/src/main/java/net/minestom/server/extensions/isolation/MinestomExtensionClassLoader.java b/src/main/java/net/minestom/server/extensions/isolation/MinestomExtensionClassLoader.java index 8918882e1..86d45ebc4 100644 --- a/src/main/java/net/minestom/server/extensions/isolation/MinestomExtensionClassLoader.java +++ b/src/main/java/net/minestom/server/extensions/isolation/MinestomExtensionClassLoader.java @@ -9,7 +9,6 @@ import org.slf4j.LoggerFactory; import java.net.URL; public class MinestomExtensionClassLoader extends HierarchyClassLoader { - private final Logger logger = LoggerFactory.getLogger(MinestomExtensionClassLoader.class); public MinestomExtensionClassLoader(String extensionName, URL[] urls) { super("Ext_" + extensionName, urls, MinecraftServer.class.getClassLoader());