Remove extension observer mechanism and cleanup/simplify classloader

This commit is contained in:
Matt Worzala 2021-11-21 15:11:50 -05:00 committed by TheMode
parent 4d089d84fb
commit 0491a63e0c
6 changed files with 21 additions and 156 deletions

View File

@ -1,7 +1,6 @@
package net.minestom.server.extensions;
import com.google.gson.JsonObject;
import net.minestom.server.extensions.isolation.MinestomExtensionClassLoader;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
@ -64,7 +63,7 @@ public final class DiscoveredExtension {
transient private Path dataDirectory;
/** The class loader that powers it. */
transient private MinestomExtensionClassLoader classLoader;
transient private ExtensionClassLoader classLoader;
@NotNull
public String getName() {
@ -116,11 +115,11 @@ public final class DiscoveredExtension {
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);
classLoader = new ExtensionClassLoader(this.getName(), urls);
}
@NotNull
public MinestomExtensionClassLoader getClassLoader() {
public ExtensionClassLoader getClassLoader() {
return classLoader;
}

View File

@ -8,19 +8,13 @@ import org.slf4j.Logger;
import java.io.IOException;
import java.io.InputStream;
import java.lang.ref.Reference;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
public abstract class Extension {
// Set by reflection
@ -33,15 +27,6 @@ public abstract class Extension {
@SuppressWarnings("unused")
private EventNode<Event> eventNode;
/**
* Observers that will be notified of events related to this extension.
* Kept as WeakReference because entities can be observers, but could become candidate to be garbage-collected while
* this extension holds a reference to it. A WeakReference makes sure this extension does not prevent the memory
* from being cleaned up.
*/
protected final Set<WeakReference<IExtensionObserver>> observers = Collections.newSetFromMap(new ConcurrentHashMap<>());
protected final ReferenceQueue<IExtensionObserver> observerReferenceQueue = new ReferenceQueue<>();
/**
* List of extensions that depend on this extension.
*/
@ -202,42 +187,6 @@ public abstract class Extension {
}
}
/**
* Adds a new observer to this extension.
* Will be kept as a WeakReference.
*
* @param observer The observer to add
*/
public void observe(IExtensionObserver observer) {
observers.add(new WeakReference<>(observer, observerReferenceQueue));
}
/**
* Calls some action on all valid observers of this extension
*
* @param action code to execute on each observer
*/
public void triggerChange(Consumer<IExtensionObserver> action) {
for (WeakReference<IExtensionObserver> weakObserver : observers) {
IExtensionObserver observer = weakObserver.get();
if (observer != null) {
action.accept(observer);
}
}
}
/**
* Removes all expired reference to observers
*
* @see #observers
*/
public void cleanupObservers() {
Reference<? extends IExtensionObserver> ref;
while ((ref = observerReferenceQueue.poll()) != null) {
observers.remove(ref);
}
}
/**
* @return A modifiable list of dependents.
*/

View File

@ -1,5 +1,6 @@
package net.minestom.server.extensions.isolation;
package net.minestom.server.extensions;
import net.minestom.server.MinecraftServer;
import org.jetbrains.annotations.NotNull;
import java.io.InputStream;
@ -8,14 +9,15 @@ 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 final class ExtensionClassLoader extends URLClassLoader {
private final List<ExtensionClassLoader> children = new LinkedList<>();
public HierarchyClassLoader(String name, URL[] urls, ClassLoader parent) {
super(name, urls, parent);
public ExtensionClassLoader(String name, URL[] urls) {
super("Ext_" + name, urls, MinecraftServer.class.getClassLoader());
}
public ExtensionClassLoader(String name, URL[] urls, ClassLoader parent) {
super("Ext_" + name, urls, parent);
}
@Override
@ -23,7 +25,7 @@ public abstract class HierarchyClassLoader extends URLClassLoader {
super.addURL(url);
}
public void addChild(@NotNull MinestomExtensionClassLoader loader) {
public void addChild(@NotNull ExtensionClassLoader loader) {
children.add(loader);
}
@ -32,7 +34,7 @@ public abstract class HierarchyClassLoader extends URLClassLoader {
try {
return super.loadClass(name, resolve);
} catch (ClassNotFoundException e) {
for (MinestomExtensionClassLoader child : children) {
for (ExtensionClassLoader child : children) {
try {
return child.loadClass(name, resolve);
} catch (ClassNotFoundException ignored) {}
@ -45,7 +47,7 @@ public abstract class HierarchyClassLoader extends URLClassLoader {
InputStream in = getResourceAsStream(name);
if (in != null) return in;
for (MinestomExtensionClassLoader child : children) {
for (ExtensionClassLoader child : children) {
InputStream childInput = child.getResourceAsStreamWithChildren(name);
if (childInput != null)
return childInput;
@ -53,13 +55,4 @@ public abstract class HierarchyClassLoader extends URLClassLoader {
return null;
}
public void removeChildInHierarchy(MinestomExtensionClassLoader child) {
children.remove(child);
// Also remove all children from these extension's children.
for (MinestomExtensionClassLoader subChild : children) {
subChild.removeChildInHierarchy(child);
}
}
}

View File

@ -7,9 +7,7 @@ import net.minestom.dependencies.maven.MavenRepository;
import net.minestom.server.MinecraftServer;
import net.minestom.server.event.Event;
import net.minestom.server.event.EventNode;
import net.minestom.server.extensions.isolation.MinestomExtensionClassLoader;
import net.minestom.server.ping.ResponseDataConsumer;
import net.minestom.server.utils.time.TimeUnit;
import net.minestom.server.utils.validate.Check;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@ -168,15 +166,6 @@ public class ExtensionManager {
}
}
//TODO(mattw) What is the purpose of this?
// Periodically cleanup observers
MinecraftServer.getSchedulerManager().buildTask(() -> {
for (Extension ext : extensions.values()) {
ext.cleanupObservers();
}
}).repeat(1L, TimeUnit.MINUTE).schedule();
// Load extensions
{
// Get all extensions and order them accordingly.
@ -242,7 +231,7 @@ public class ExtensionManager {
String extensionName = discoveredExtension.getName();
String mainClass = discoveredExtension.getEntrypoint();
MinestomExtensionClassLoader loader = discoveredExtension.getClassLoader();
ExtensionClassLoader loader = discoveredExtension.getClassLoader();
if (extensions.containsKey(extensionName.toLowerCase())) {
LOGGER.error("An extension called '{}' has already been registered.", extensionName);
@ -588,16 +577,14 @@ public class ExtensionManager {
LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved);
}
MinestomExtensionClassLoader extensionClassLoader = discoveredExtension.getClassLoader();
ExtensionClassLoader extensionClassLoader = discoveredExtension.getClassLoader();
for (String dependencyName : discoveredExtension.getDependencies()) {
var resolved = extensions.stream()
.filter(ext -> ext.getName().equalsIgnoreCase(dependencyName))
.findFirst().orElse(null);
if (resolved == null) {
throw new IllegalStateException("Unknown dependency '" + dependencyName + "' of '" + discoveredExtension.getName() + "'");
}
.findFirst()
.orElseThrow(() -> new IllegalStateException("Unknown dependency '" + dependencyName + "' of '" + discoveredExtension.getName() + "'"));
MinestomExtensionClassLoader dependencyClassLoader = resolved.getClassLoader();
ExtensionClassLoader dependencyClassLoader = resolved.getClassLoader();
extensionClassLoader.addChild(dependencyClassLoader);
LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved);
@ -700,9 +687,6 @@ public class ExtensionManager {
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));
// Remove event node
EventNode<Event> eventNode = ext.getEventNode();

View File

@ -1,14 +0,0 @@
package net.minestom.server.extensions;
/**
* Observes events related to extensions
*/
public interface IExtensionObserver {
/**
* Called before unloading an extension (that is, right after Extension#terminate and right before Extension#unload)
* @param extensionName the name of the extension that is being unloaded
*/
void onExtensionUnload(String extensionName);
}

View File

@ -1,46 +0,0 @@
package net.minestom.server.extensions.isolation;
import net.minestom.server.MinecraftServer;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.net.URL;
public class MinestomExtensionClassLoader extends HierarchyClassLoader {
public MinestomExtensionClassLoader(String extensionName, URL[] urls) {
super("Ext_" + extensionName, urls, MinecraftServer.class.getClassLoader());
}
/**
* Returns the name of the extension linked to this classloader
* @return the name of the extension linked to this classloader
*/
public String getExtensionName() {
// simply calls ClassLoader#getName as the extension name is used to name this classloader
// this method is simply for ease-of-use
return getName();
}
/**
* Tries to know which extension created this object, based on the classloader of the object. This can only check that the class of the object has been loaded
* by an extension.
*
* While not perfect, this should detect any callback created via extension code.
* It is possible this current version of the implementation might struggle with callbacks created through external
* libraries, but as libraries are loaded separately for each extension, this *should not*(tm) be a problem.
*
* @param obj the object to get the extension of
* @return <code>null</code> if no extension has been found, otherwise the extension name
*/
@Nullable
public static String findExtensionObjectOwner(@NotNull Object obj) {
ClassLoader cl = obj.getClass().getClassLoader();
if (cl instanceof MinestomExtensionClassLoader extensionClassLoader) {
return extensionClassLoader.getExtensionName();
}
return null;
}
}