Upstream
/
Minestom
mirror of https://github.com/Minestom/Minestom.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Stop using ExtensionDependencyResolver, load external dependencies correctly

This commit is contained in:
Matt Worzala 2021-11-16 12:08:15 -05:00 committed by TheMode
parent 9f5122019f
commit 58e5bab5da
4 changed files with 52 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/main/java/net/minestom/server/extensions/DiscoveredExtension.java
View File

@ -206,7 +206,7 @@ public final class DiscoveredExtension {
return meta;
}
public MinestomExtensionClassLoader makeClassLoader(List<DiscoveredExtension> discoveredExtensions) {
public MinestomExtensionClassLoader makeClassLoader() {
final URL[] urls = this.files.toArray(new URL[0]);
MinestomExtensionClassLoader loader = new MinestomExtensionClassLoader(this.getName(), urls);
@ -214,21 +214,21 @@ public final class DiscoveredExtension {
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);
}
}
// 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

59
src/main/java/net/minestom/server/extensions/ExtensionDependencyResolver.java
View File

@ -1,59 +0,0 @@
package net.minestom.server.extensions;
import net.minestom.dependencies.DependencyResolver;
import net.minestom.dependencies.ResolvedDependency;
import net.minestom.dependencies.UnresolvedDependencyException;
import org.jetbrains.annotations.NotNull;
import java.io.File;
import java.net.URL;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
/**
* Does NOT relocate extensions
*/
public class ExtensionDependencyResolver implements DependencyResolver {
private final Map<String, DiscoveredExtension> extensionMap = new HashMap<>();
public ExtensionDependencyResolver(@NotNull List<DiscoveredExtension> extensions) {
for (DiscoveredExtension ext : extensions) {
extensionMap.put(ext.getName(), ext);
}
}
@NotNull
@Override
public ResolvedDependency resolve(@NotNull String extensionName, @NotNull File file) throws UnresolvedDependencyException {
if (extensionMap.containsKey(extensionName)) {
DiscoveredExtension ext = extensionMap.get(extensionName);
// convert extension URLs to subdependencies
// FIXME: this is not a deep conversion, this might create an issue in this scenario with different classloaders:
// A depends on an external lib (Ext<-A)
// B depends on A (A<-B)
// When loading B, with no deep conversion, Ext will not be added to the list of dependencies (because it is not a direct dependency)
// But when trying to call/access code from extension A, the parts dependent on Ext won't be inside B's dependencies, triggering a ClassNotFoundException
List<ResolvedDependency> dependencies = new LinkedList<>();
for (URL url : ext.files) {
dependencies.add(new ResolvedDependency(url.toExternalForm(), url.toExternalForm(), "", url, new LinkedList<>()));
}
return new ResolvedDependency(ext.getName(), ext.getName(), ext.getVersion(), ext.files.get(0), dependencies);
}
throw new UnresolvedDependencyException("No extension named " + extensionName);
}
@Override
public String toString() {
String list = extensionMap.values()
.stream()
.map(DiscoveredExtension::getName)
.collect(Collectors.joining(", "));
return "ExtensionDependencyResolver[" + list + "]";
}
}

58
src/main/java/net/minestom/server/extensions/ExtensionManager.java
View File

@ -168,6 +168,7 @@ public class ExtensionManager {
}
}
//TODO(mattw) What is the purpose of this?
// Periodically cleanup observers
MinecraftServer.getSchedulerManager().buildTask(() -> {
for (Extension ext : extensions.values()) {
@ -183,25 +184,26 @@ public class ExtensionManager {
// Don't waste resources on doing extra actions if there is nothing to do.
if (discoveredExtensions.isEmpty()) return;
discoveredExtensions = generateLoadOrder(discoveredExtensions);
loadDependencies(discoveredExtensions);
// remove invalid extensions
discoveredExtensions.removeIf(ext -> ext.loadStatus != DiscoveredExtension.LoadStatus.LOAD_SUCCESS);
// set class loaders for all extensions.
for (DiscoveredExtension discoveredExtension : discoveredExtensions) {
// Create classloaders for each extension (so that they can be used during dependency resolution)
Iterator<DiscoveredExtension> extensionIterator = discoveredExtensions.iterator();
while (extensionIterator.hasNext()) {
DiscoveredExtension discoveredExtension = extensionIterator.next();
try {
discoveredExtension.setMinestomExtensionClassLoader(discoveredExtension.makeClassLoader(discoveredExtensions)); //TODO: This is a hack to pass the dependent DiscoveredExtensions to the classloader
MinestomExtensionClassLoader classLoader = discoveredExtension.makeClassLoader();
discoveredExtension.setMinestomExtensionClassLoader(classLoader);
System.out.println("SET " + discoveredExtension.getName() + " TO " + discoveredExtension.getMinestomExtensionClassLoader());
} catch (Exception e) {
discoveredExtension.loadStatus = DiscoveredExtension.LoadStatus.FAILED_TO_SETUP_CLASSLOADER;
MinecraftServer.getExceptionManager().handleException(e);
LOGGER.error("Failed to load extension {}", discoveredExtension.getName());
LOGGER.error("Failed to load extension", e);
extensionIterator.remove();
}
}
discoveredExtensions = generateLoadOrder(discoveredExtensions);
loadDependencies(discoveredExtensions);
// remove invalid extensions
discoveredExtensions.removeIf(ext -> ext.loadStatus != DiscoveredExtension.LoadStatus.LOAD_SUCCESS);
@ -382,6 +384,9 @@ public class ExtensionManager {
}
}
//TODO(mattw): Extract this into its own method to load an extension given classes and resources directory.
//TODO(mattw): Should show a warning if one is set and not the other. It is most likely a mistake.
// this allows developers to have their extension discovered while working on it, without having to build a jar and put in the extension folder
if (System.getProperty(INDEV_CLASSES_FOLDER) != null && System.getProperty(INDEV_RESOURCES_FOLDER) != null) {
LOGGER.info("Found indev folders for extension. Adding to list of discovered extensions.");
@ -558,8 +563,6 @@ public class ExtensionManager {
for (Extension extension : immutableExtensions.values())
allLoadedExtensions.add(extension.getOrigin());
ExtensionDependencyResolver extensionDependencyResolver = new ExtensionDependencyResolver(allLoadedExtensions);
for (DiscoveredExtension discoveredExtension : extensions) {
try {
DependencyGetter getter = new DependencyGetter();
@ -567,27 +570,18 @@ public class ExtensionManager {
List<MavenRepository> repoList = new LinkedList<>();
for (var repository : externalDependencies.repositories) {
if (repository.name == null) {
if (repository.name == null || repository.name.isEmpty()) {
throw new IllegalStateException("Missing 'name' element in repository object.");
}
if (repository.name.isEmpty()) {
throw new IllegalStateException("Invalid 'name' element in repository object.");
}
if (repository.url == null) {
if (repository.url == null || repository.url.isEmpty()) {
throw new IllegalStateException("Missing 'url' element in repository object.");
}
if (repository.url.isEmpty()) {
throw new IllegalStateException("Invalid 'url' element in repository object.");
}
repoList.add(new MavenRepository(repository.name, repository.url));
}
getter.addMavenResolver(repoList);
// getter.addResolver(extensionDependencyResolver);
for (String artifact : externalDependencies.artifacts) {
var resolved = getter.get(artifact, dependenciesFolder);
@ -595,11 +589,20 @@ public class ExtensionManager {
LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved);
}
// for (String dependencyName : discoveredExtension.getDependencies()) {
// var resolved = getter.get(dependencyName, dependenciesFolder);
// addDependencyFile(resolved, discoveredExtension);
// LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved);
// }
MinestomExtensionClassLoader extensionClassLoader = discoveredExtension.getMinestomExtensionClassLoader();
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.");
}
extensionClassLoader.addChild(dependencyClassLoader);
LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved);
}
} catch (Exception e) {
discoveredExtension.loadStatus = DiscoveredExtension.LoadStatus.MISSING_DEPENDENCIES;
LOGGER.error("Failed to load dependencies for extension {}", discoveredExtension.getName());
@ -613,6 +616,7 @@ public class ExtensionManager {
System.out.println("ADDING DEPENDENCY " + dependency.getName() + " TO " + extension.getName());
URL location = dependency.getContentsLocation();
extension.files.add(location);
extension.getMinestomExtensionClassLoader().addURL(location);
LOGGER.trace("Added dependency {} to extension {} classpath", location.toExternalForm(), extension.getName());
// recurse to add full dependency tree

5
src/main/java/net/minestom/server/extensions/isolation/HierarchyClassLoader.java
View File

@ -18,6 +18,11 @@ public abstract class HierarchyClassLoader extends URLClassLoader {
super(name, urls, parent);
}
@Override
public void addURL(@NotNull URL url) {
super.addURL(url);
}
public void addChild(@NotNull MinestomExtensionClassLoader loader) {
children.add(loader);
}