Mixin modifications between extensions is now possible

src/main/java/net/minestom/server/extensions/DiscoveredExtension.java

@@ -129,6 +129,8 @@ final class DiscoveredExtension {
         MISSING_DEPENDENCIES("Missing dependencies, check your logs."),
         INVALID_NAME("Invalid name."),
         NO_ENTRYPOINT("No entrypoint specified."),
+        FAILED_TO_SETUP_CLASSLOADER("Extension classloader could not be setup."),
+        LOAD_FAILED("Load failed. See logs for more information."),
         ;
 
         private final String message;

src/main/java/net/minestom/server/extensions/ExtensionManager.java

@@ -33,7 +33,7 @@ public class ExtensionManager {
     private final static String INDEV_RESOURCES_FOLDER = "minestom.extension.indevfolder.resources";
     private final static Gson GSON = new Gson();
 
-    private final Map<String, URLClassLoader> extensionLoaders = new HashMap<>();
+    private final Map<String, MinestomExtensionClassLoader> extensionLoaders = new HashMap<>();
     private final Map<String, Extension> extensions = new HashMap<>();
     private final File extensionFolder = new File("extensions");
     private final File dependenciesFolder = new File(extensionFolder, ".libs");
@@ -66,109 +66,134 @@ public class ExtensionManager {
         List<DiscoveredExtension> discoveredExtensions = discoverExtensions();
         discoveredExtensions = generateLoadOrder(discoveredExtensions);
         loadDependencies(discoveredExtensions);
+        // remove invalid extensions
+        discoveredExtensions.removeIf(ext -> ext.loadStatus != DiscoveredExtension.LoadStatus.LOAD_SUCCESS);
+
+        for(DiscoveredExtension discoveredExtension : discoveredExtensions) {
+            try {
+                setupClassLoader(discoveredExtension);
+            } catch (Exception e) {
+                discoveredExtension.loadStatus = DiscoveredExtension.LoadStatus.FAILED_TO_SETUP_CLASSLOADER;
+                e.printStackTrace();
+                log.error("Failed to load extension {}", discoveredExtension.getName());
+                log.error("Failed to load extension", e);
+            }
+        }
+
         // remove invalid extensions
         discoveredExtensions.removeIf(ext -> ext.loadStatus != DiscoveredExtension.LoadStatus.LOAD_SUCCESS);
         setupCodeModifiers(discoveredExtensions);
 
         for (DiscoveredExtension discoveredExtension : discoveredExtensions) {
-            URLClassLoader loader;
-            URL[] urls = discoveredExtension.files.toArray(new URL[0]);
-            // TODO: Only putting each extension into its own classloader prevents code modifications (via code modifiers or mixins)
-            // TODO: If we want modifications to be possible, we need to add these urls to the current classloader
-            // TODO: Indeed, without adding the urls, the classloader is not able to load the bytecode of extension classes
-            // TODO: Whether we want to allow extensions to modify one-another is our choice now.
-            loader = newClassLoader(discoveredExtension, urls);
-
-            // Create ExtensionDescription (authors, version etc.)
-            String extensionName = discoveredExtension.getName();
-            String mainClass = discoveredExtension.getEntrypoint();
-            Extension.ExtensionDescription extensionDescription = new Extension.ExtensionDescription(
-                    extensionName,
-                    discoveredExtension.getVersion(),
-                    Arrays.asList(discoveredExtension.getAuthors())
-            );
-
-            extensionLoaders.put(extensionName.toLowerCase(), loader);
-
-            if (extensions.containsKey(extensionName.toLowerCase())) {
-                log.error("An extension called '{}' has already been registered.", extensionName);
-                continue;
-            }
-
-            Class<?> jarClass;
             try {
-                jarClass = Class.forName(mainClass, true, loader);
+                attemptSingleLoad(discoveredExtension);
-            } catch (ClassNotFoundException e) {
+            } catch (Exception e) {
-                log.error("Could not find main class '{}' in extension '{}'.", mainClass, extensionName, e);
+                discoveredExtension.loadStatus = DiscoveredExtension.LoadStatus.LOAD_FAILED;
-                continue;
-            }
-
-            Class<? extends Extension> extensionClass;
-            try {
-                extensionClass = jarClass.asSubclass(Extension.class);
-            } catch (ClassCastException e) {
-                log.error("Main class '{}' in '{}' does not extend the 'Extension' superclass.", mainClass, extensionName, e);
-                continue;
-            }
-
-            Constructor<? extends Extension> constructor;
-            try {
-                constructor = extensionClass.getDeclaredConstructor();
-                // Let's just make it accessible, plugin creators don't have to make this public.
-                constructor.setAccessible(true);
-            } catch (NoSuchMethodException e) {
-                log.error("Main class '{}' in '{}' does not define a no-args constructor.", mainClass, extensionName, e);
-                continue;
-            }
-            Extension extension = null;
-            try {
-                extension = constructor.newInstance();
-            } catch (InstantiationException e) {
-                log.error("Main class '{}' in '{}' cannot be an abstract class.", mainClass, extensionName, e);
-                continue;
-            } catch (IllegalAccessException ignored) {
-                // We made it accessible, should not occur
-            } catch (InvocationTargetException e) {
-                log.error(
-                        "While instantiating the main class '{}' in '{}' an exception was thrown.",
-                        mainClass,
-                        extensionName,
-                        e.getTargetException()
-                );
-                continue;
-            }
-
-            // Set extension description
-            try {
-                Field descriptionField = extensionClass.getSuperclass().getDeclaredField("description");
-                descriptionField.setAccessible(true);
-                descriptionField.set(extension, extensionDescription);
-            } catch (IllegalAccessException e) {
-                // We made it accessible, should not occur
-            } catch (NoSuchFieldException e) {
-                log.error("Main class '{}' in '{}' has no description field.", mainClass, extensionName, e);
-                continue;
-            }
-
-            // Set logger
-            try {
-                Field loggerField = extensionClass.getSuperclass().getDeclaredField("logger");
-                loggerField.setAccessible(true);
-                loggerField.set(extension, LoggerFactory.getLogger(extensionClass));
-            } catch (IllegalAccessException e) {
-                // We made it accessible, should not occur
                 e.printStackTrace();
-            } catch (NoSuchFieldException e) {
+                log.error("Failed to load extension {}", discoveredExtension.getName());
-                // This should also not occur (unless someone changed the logger in Extension superclass).
+                log.error("Failed to load extension", e);
-                log.error("Main class '{}' in '{}' has no logger field.", mainClass, extensionName, e);
             }
-
-            extensionList.add(extension); // add to a list, as lists preserve order
-            extensions.put(extensionName.toLowerCase(), extension);
         }
         extensionList = Collections.unmodifiableList(extensionList);
     }
 
+    private void setupClassLoader(DiscoveredExtension discoveredExtension) {
+        String extensionName = discoveredExtension.getName();
+        MinestomExtensionClassLoader loader;
+        URL[] urls = discoveredExtension.files.toArray(new URL[0]);
+        loader = newClassLoader(discoveredExtension, urls);
+        extensionLoaders.put(extensionName.toLowerCase(), loader);
+    }
+
+    private void attemptSingleLoad(DiscoveredExtension discoveredExtension) {
+        // Create ExtensionDescription (authors, version etc.)
+        String extensionName = discoveredExtension.getName();
+        String mainClass = discoveredExtension.getEntrypoint();
+        Extension.ExtensionDescription extensionDescription = new Extension.ExtensionDescription(
+                extensionName,
+                discoveredExtension.getVersion(),
+                Arrays.asList(discoveredExtension.getAuthors())
+        );
+
+        MinestomExtensionClassLoader loader = extensionLoaders.get(extensionName.toLowerCase());
+
+        if (extensions.containsKey(extensionName.toLowerCase())) {
+            log.error("An extension called '{}' has already been registered.", extensionName);
+            return;
+        }
+
+        Class<?> jarClass;
+        try {
+            jarClass = Class.forName(mainClass, true, loader);
+        } catch (ClassNotFoundException e) {
+            log.error("Could not find main class '{}' in extension '{}'.", mainClass, extensionName, e);
+            return;
+        }
+
+        Class<? extends Extension> extensionClass;
+        try {
+            extensionClass = jarClass.asSubclass(Extension.class);
+        } catch (ClassCastException e) {
+            log.error("Main class '{}' in '{}' does not extend the 'Extension' superclass.", mainClass, extensionName, e);
+            return;
+        }
+
+        Constructor<? extends Extension> constructor;
+        try {
+            constructor = extensionClass.getDeclaredConstructor();
+            // Let's just make it accessible, plugin creators don't have to make this public.
+            constructor.setAccessible(true);
+        } catch (NoSuchMethodException e) {
+            log.error("Main class '{}' in '{}' does not define a no-args constructor.", mainClass, extensionName, e);
+            return;
+        }
+        Extension extension = null;
+        try {
+            extension = constructor.newInstance();
+        } catch (InstantiationException e) {
+            log.error("Main class '{}' in '{}' cannot be an abstract class.", mainClass, extensionName, e);
+            return;
+        } catch (IllegalAccessException ignored) {
+            // We made it accessible, should not occur
+        } catch (InvocationTargetException e) {
+            log.error(
+                    "While instantiating the main class '{}' in '{}' an exception was thrown.",
+                    mainClass,
+                    extensionName,
+                    e.getTargetException()
+            );
+            return;
+        }
+
+        // Set extension description
+        try {
+            Field descriptionField = extensionClass.getSuperclass().getDeclaredField("description");
+            descriptionField.setAccessible(true);
+            descriptionField.set(extension, extensionDescription);
+        } catch (IllegalAccessException e) {
+            // We made it accessible, should not occur
+        } catch (NoSuchFieldException e) {
+            log.error("Main class '{}' in '{}' has no description field.", mainClass, extensionName, e);
+            return;
+        }
+
+        // Set logger
+        try {
+            Field loggerField = extensionClass.getSuperclass().getDeclaredField("logger");
+            loggerField.setAccessible(true);
+            loggerField.set(extension, LoggerFactory.getLogger(extensionClass));
+        } catch (IllegalAccessException e) {
+            // We made it accessible, should not occur
+            e.printStackTrace();
+        } catch (NoSuchFieldException e) {
+            // This should also not occur (unless someone changed the logger in Extension superclass).
+            log.error("Main class '{}' in '{}' has no logger field.", mainClass, extensionName, e);
+        }
+
+        extensionList.add(extension); // add to a list, as lists preserve order
+        extensions.put(extensionName.toLowerCase(), extension);
+    }
+
     @NotNull
     private List<DiscoveredExtension> discoverExtensions() {
         List<DiscoveredExtension> extensions = new LinkedList<>();
@@ -334,42 +359,41 @@ public class ExtensionManager {
         }
     }
 
-    // TODO: remove if extensions cannot modify one-another
-    // TODO: use if they can
-    private void injectIntoClasspath(URL dependency, DiscoveredExtension extension) {
-        final ClassLoader cl = getClass().getClassLoader();
-        if (!(cl instanceof URLClassLoader)) {
-            throw new IllegalStateException("Current class loader is not a URLClassLoader, but " + cl + ". This prevents adding URLs into the classpath at runtime.");
-        }
-        if(cl instanceof MinestomRootClassLoader) {
-            ((MinestomRootClassLoader) cl).addURL(dependency); // no reflection warnings for us!
-        } else {
-            try {
-                Method addURL = URLClassLoader.class.getDeclaredMethod("addURL", URL.class);
-                addURL.setAccessible(true);
-                addURL.invoke(cl, dependency);
-            } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
-                throw new RuntimeException("Failed to inject URL " + dependency + " into classpath. From extension " + extension.getName(), e);
-            }
-        }
-    }
-
     private void addDependencyFile(URL dependency, DiscoveredExtension extension) {
         extension.files.add(dependency);
         log.trace("Added dependency {} to extension {} classpath", dependency.toExternalForm(), extension.getName());
     }
 
     /**
-     * Loads a URL into the classpath.
+     * Creates a new class loader for the given extension.
+     * Will add the new loader as a child of all its dependencies' loaders.
      *
      * @param urls {@link URL} (usually a JAR) that should be loaded.
      */
     @NotNull
-    public URLClassLoader newClassLoader(@NotNull DiscoveredExtension extension, @NotNull URL[] urls) {
+    public MinestomExtensionClassLoader newClassLoader(@NotNull DiscoveredExtension extension, @NotNull URL[] urls) {
         MinestomRootClassLoader root = MinestomRootClassLoader.getInstance();
         MinestomExtensionClassLoader loader = new MinestomExtensionClassLoader(extension.getName(), urls, root);
-        // TODO: tree structure
+        if(extension.getDependencies().length == 0) {
-        root.addChild(loader);
+            // orphaned extension, we can insert it directly
+            root.addChild(loader);
+        } else {
+            // we need to keep track that it has actually been inserted
+            // even though it should always be (due to the order in which extensions are loaders), it is an additional layer of """security"""
+            boolean foundOne = false;
+            for(String dependency : extension.getDependencies()) {
+                if(extensionLoaders.containsKey(dependency.toLowerCase())) {
+                    MinestomExtensionClassLoader parentLoader = extensionLoaders.get(dependency.toLowerCase());
+                    parentLoader.addChild(loader);
+                    foundOne = true;
+                }
+            }
+
+            if(!foundOne) {
+                log.error("Could not load extension {}, could not find any parent inside classloader hierarchy.", extension.getName());
+                throw new RuntimeException("Could not load extension "+extension.getName()+", could not find any parent inside classloader hierarchy.");
+            }
+        }
         return loader;
     }
 

src/main/java/net/minestom/server/extras/selfmodification/HierarchyClassLoader.java

@@ -0,0 +1,36 @@
+package net.minestom.server.extras.selfmodification;
+
+import org.jetbrains.annotations.NotNull;
+
+import java.io.InputStream;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.LinkedList;
+import java.util.List;
+
+/**
+ * Classloader part of a hierarchy of classloader
+ */
+public abstract class HierarchyClassLoader extends URLClassLoader {
+    protected final List<MinestomExtensionClassLoader> children = new LinkedList<>();
+
+    public HierarchyClassLoader(String name, URL[] urls, ClassLoader parent) {
+        super(name, urls, parent);
+    }
+
+    public void addChild(@NotNull MinestomExtensionClassLoader loader) {
+        children.add(loader);
+    }
+
+    public InputStream getResourceAsStreamWithChildren(String name) {
+        InputStream in = getResourceAsStream(name);
+        if(in != null) return in;
+
+        for(MinestomExtensionClassLoader child : children) {
+            InputStream childInput = child.getResourceAsStreamWithChildren(name);
+            if(childInput != null)
+                return childInput;
+        }
+        return null;
+    }
+}

src/main/java/net/minestom/server/extras/selfmodification/MinestomExtensionClassLoader.java

@@ -3,16 +3,12 @@ package net.minestom.server.extras.selfmodification;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
-import java.net.URLClassLoader;
-import java.util.LinkedList;
-import java.util.List;
 
-public class MinestomExtensionClassLoader extends URLClassLoader {
+public class MinestomExtensionClassLoader extends HierarchyClassLoader {
     /**
      * Root ClassLoader, everything goes through it before any attempt at loading is done inside this classloader
      */
     private final MinestomRootClassLoader root;
-    private final List<MinestomExtensionClassLoader> children = new LinkedList<>();
 
     public MinestomExtensionClassLoader(String name, URL[] urls, MinestomRootClassLoader root) {
         super(name, urls, root);
@@ -36,35 +32,39 @@ public class MinestomExtensionClassLoader extends URLClassLoader {
      * @throws ClassNotFoundException if the class is not found inside this classloader
      */
     public Class<?> loadClassAsChild(String name, boolean resolve) throws ClassNotFoundException {
-        for(MinestomExtensionClassLoader child : children) {
-            try {
-                Class<?> loaded = child.loadClassAsChild(name, resolve);
-                return loaded;
-            } catch (ClassNotFoundException e) {
-                // move on to next child
-            }
-        }
-
         Class<?> loadedClass = findLoadedClass(name);
         if(loadedClass != null) {
             return loadedClass;
         }
-        // not in children, attempt load in this classloader
+
-        String path = name.replace(".", "/") + ".class";
+        try {
-        InputStream in = getResourceAsStream(path);
+            // not in children, attempt load in this classloader
-        if(in == null) {
+            String path = name.replace(".", "/") + ".class";
-            throw new ClassNotFoundException("Could not load class "+name);
+            InputStream in = getResourceAsStream(path);
-        }
+            if (in == null) {
-        try(in) {
+                throw new ClassNotFoundException("Could not load class " + name);
-            byte[] bytes = in.readAllBytes();
-            bytes = root.transformBytes(bytes, name);
-            Class<?> clazz = defineClass(name, bytes, 0, bytes.length);
-            if(resolve) {
-                resolveClass(clazz);
             }
-            return clazz;
+            try (in) {
-        } catch (IOException e) {
+                byte[] bytes = in.readAllBytes();
-            throw new ClassNotFoundException("Could not load class "+name, e);
+                bytes = root.transformBytes(bytes, name);
+                Class<?> clazz = defineClass(name, bytes, 0, bytes.length);
+                if (resolve) {
+                    resolveClass(clazz);
+                }
+                return clazz;
+            } catch (IOException e) {
+                throw new ClassNotFoundException("Could not load class " + name, e);
+            }
+        } catch (ClassNotFoundException e) {
+            for(MinestomExtensionClassLoader child : children) {
+                try {
+                    Class<?> loaded = child.loadClassAsChild(name, resolve);
+                    return loaded;
+                } catch (ClassNotFoundException e1) {
+                    // move on to next child
+                }
+            }
+            throw e;
         }
     }
 }

src/main/java/net/minestom/server/extras/selfmodification/MinestomRootClassLoader.java

@@ -22,7 +22,7 @@ import java.util.Set;
  * Class Loader that can modify class bytecode when they are loaded
  */
 @Slf4j
-public class MinestomRootClassLoader extends URLClassLoader {
+public class MinestomRootClassLoader extends HierarchyClassLoader {
 
     private static MinestomRootClassLoader INSTANCE;
 
@@ -60,7 +60,6 @@ public class MinestomRootClassLoader extends URLClassLoader {
 
     // TODO: priorities?
     private final List<CodeModifier> modifiers = new LinkedList<>();
-    private final List<MinestomExtensionClassLoader> children = new LinkedList<>();
 
     private MinestomRootClassLoader(ClassLoader parent) {
         super("Minestom Root ClassLoader", extractURLsFromClasspath(), parent);
@@ -187,6 +186,21 @@ public class MinestomRootClassLoader extends URLClassLoader {
         return originalBytes;
     }
 
+    public byte[] loadBytesWithChildren(String name, boolean transform) throws IOException, ClassNotFoundException {
+        if (name == null)
+            throw new ClassNotFoundException();
+        String path = name.replace(".", "/") + ".class";
+        InputStream input = getResourceAsStreamWithChildren(path);
+        if(input == null) {
+            throw new ClassNotFoundException("Could not find resource "+path);
+        }
+        byte[] originalBytes = input.readAllBytes();
+        if(transform) {
+            return transformBytes(originalBytes, name);
+        }
+        return originalBytes;
+    }
+
     byte[] transformBytes(byte[] classBytecode, String name) {
         if (!isProtected(name)) {
             ClassReader reader = new ClassReader(classBytecode);
@@ -261,8 +275,4 @@ public class MinestomRootClassLoader extends URLClassLoader {
     public List<CodeModifier> getModifiers() {
         return modifiers;
     }
-
-    public void addChild(MinestomExtensionClassLoader loader) {
-        children.add(loader);
-    }
 }

src/main/java/net/minestom/server/extras/selfmodification/mixins/MinestomBytecodeProvider.java

@@ -26,7 +26,7 @@ public class MinestomBytecodeProvider implements IClassBytecodeProvider {
         ClassNode node = new ClassNode();
         ClassReader reader;
         try {
-            reader = new ClassReader(classLoader.loadBytes(name, transform));
+            reader = new ClassReader(classLoader.loadBytesWithChildren(name, transform));
         } catch (IOException e) {
             throw new ClassNotFoundException("Could not load ClassNode with name " + name, e);
         }

src/main/java/net/minestom/server/extras/selfmodification/mixins/MixinServiceMinestom.java

@@ -65,7 +65,7 @@ public class MixinServiceMinestom extends MixinServiceAbstract {
 
     @Override
     public InputStream getResourceAsStream(String name) {
-        return classLoader.getResourceAsStream(name);
+        return classLoader.getResourceAsStreamWithChildren(name);
     }
 
     @Override