Don't log or die on cyclic dependencies of Spigot plugins

This commit is contained in:
Nassim Jahnke 2023-02-27 12:09:10 +01:00
parent 9256248567
commit ad3e1bc121
No known key found for this signature in database
GPG key ID: 6BE3B555EBC5982B

View file

@ -1771,10 +1771,10 @@ index 0000000000000000000000000000000000000000..6f201a8131ca9631ac4af62c75e6f2e8
+}
diff --git a/src/main/java/io/papermc/paper/plugin/entrypoint/strategy/JohnsonSimpleCycles.java b/src/main/java/io/papermc/paper/plugin/entrypoint/strategy/JohnsonSimpleCycles.java
new file mode 100644
index 0000000000000000000000000000000000000000..22189a1c42459c00d3e8bdeb980d15a69b720805
index 0000000000000000000000000000000000000000..a9bca905eba67972e4d1b07b1d243272b62fec66
--- /dev/null
+++ b/src/main/java/io/papermc/paper/plugin/entrypoint/strategy/JohnsonSimpleCycles.java
@@ -0,0 +1,350 @@
@@ -0,0 +1,354 @@
+/*
+ * (C) Copyright 2013-2021, by Nikolay Ognyanov and Contributors.
+ *
@ -1811,6 +1811,7 @@ index 0000000000000000000000000000000000000000..22189a1c42459c00d3e8bdeb980d15a6
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+
+/**
@ -1832,6 +1833,7 @@ index 0000000000000000000000000000000000000000..22189a1c42459c00d3e8bdeb980d15a6
+
+ // The main state of the algorithm.
+ private Consumer<List<V>> cycleConsumer = null;
+ private BiConsumer<V, V> cycleVertexSuccessorConsumer = null; // Paper
+ private V[] iToV = null;
+ private Map<V, Integer> vToI = null;
+ private Set<V> blocked = null;
@ -1865,10 +1867,10 @@ index 0000000000000000000000000000000000000000..22189a1c42459c00d3e8bdeb980d15a6
+ *
+ * @return The list of all simple cycles. Possibly empty but never <code>null</code>.
+ */
+ public List<List<V>> findSimpleCycles()
+ public List<List<V>> findAndRemoveSimpleCycles()
+ {
+ List<List<V>> result = new ArrayList<>();
+ findSimpleCycles(result::add);
+ findSimpleCycles(result::add, (v, s) -> ((MutableGraph<V>) graph).removeEdge(v, s)); // Paper
+ return result;
+ }
+
@ -1877,11 +1879,12 @@ index 0000000000000000000000000000000000000000..22189a1c42459c00d3e8bdeb980d15a6
+ *
+ * @param consumer Consumer that will be called with each cycle found.
+ */
+ public void findSimpleCycles(Consumer<List<V>> consumer)
+ public void findSimpleCycles(Consumer<List<V>> consumer, BiConsumer<V, V> vertexSuccessorConsumer) // Paper
+ {
+ if (graph == null) {
+ throw new IllegalArgumentException("Null graph.");
+ }
+ cycleVertexSuccessorConsumer = vertexSuccessorConsumer; // Paper
+ initState(consumer);
+
+ int startIndex = 0;
@ -2032,7 +2035,8 @@ index 0000000000000000000000000000000000000000..22189a1c42459c00d3e8bdeb980d15a6
+ List<V> cycle = new ArrayList<>(stack.size());
+ stack.descendingIterator().forEachRemaining(cycle::add);
+ cycleConsumer.accept(cycle);
+ foundCycle = true;
+ cycleVertexSuccessorConsumer.accept(vertex, successor); // Paper
+ //foundCycle = true; // Paper
+ } else if (!blocked.contains(successor)) {
+ boolean gotCycle = findCyclesInSCG(startIndex, successorIndex, scg);
+ foundCycle = foundCycle || gotCycle;
@ -2402,10 +2406,10 @@ index 0000000000000000000000000000000000000000..779f6980a71db8df68c9cf2078497640
+}
diff --git a/src/main/java/io/papermc/paper/plugin/entrypoint/strategy/ModernPluginLoadingStrategy.java b/src/main/java/io/papermc/paper/plugin/entrypoint/strategy/ModernPluginLoadingStrategy.java
new file mode 100644
index 0000000000000000000000000000000000000000..fa7bbfeff53d5cab3d96a5f1181b610f4ce22518
index 0000000000000000000000000000000000000000..671e9ebb8246a4cc9e212f5b11ea476560f2f7d5
--- /dev/null
+++ b/src/main/java/io/papermc/paper/plugin/entrypoint/strategy/ModernPluginLoadingStrategy.java
@@ -0,0 +1,148 @@
@@ -0,0 +1,209 @@
+package io.papermc.paper.plugin.entrypoint.strategy;
+
+import com.google.common.collect.Lists;
@ -2418,6 +2422,10 @@ index 0000000000000000000000000000000000000000..fa7bbfeff53d5cab3d96a5f1181b610f
+import io.papermc.paper.plugin.entrypoint.dependency.GraphDependencyContext;
+import io.papermc.paper.plugin.provider.PluginProvider;
+import io.papermc.paper.plugin.provider.configuration.LoadOrderConfiguration;
+import io.papermc.paper.plugin.provider.configuration.PaperPluginMeta;
+import io.papermc.paper.plugin.provider.type.spigot.SpigotPluginProvider;
+import java.util.HashSet;
+import java.util.Set;
+import org.bukkit.plugin.UnknownDependencyException;
+import org.slf4j.Logger;
+
@ -2512,7 +2520,25 @@ index 0000000000000000000000000000000000000000..fa7bbfeff53d5cab3d96a5f1181b610f
+ try {
+ reversedTopographicSort = Lists.reverse(TopographicGraphSorter.sortGraph(loadOrderGraph));
+ } catch (TopographicGraphSorter.GraphCycleException exception) {
+ throw new PluginGraphCycleException(new JohnsonSimpleCycles<>(loadOrderGraph).findSimpleCycles());
+ List<List<String>> cycles = new JohnsonSimpleCycles<>(loadOrderGraph).findAndRemoveSimpleCycles();
+
+ // Only log an error if at least non-Spigot plugin is present in the cycle
+ // Due to Spigot plugin metadata making no distinction between load order and dependencies (= class loader access), cycles are an unfortunate reality we have to deal with
+ Set<String> cyclingPlugins = new HashSet<>();
+ cycles.forEach(cyclingPlugins::addAll);
+ if (cyclingPlugins.stream().anyMatch(plugin -> {
+ PluginProvider<?> pluginProvider = providerMapMirror.get(plugin);
+ return pluginProvider != null && !(pluginProvider instanceof SpigotPluginProvider);
+ })) {
+ logCycleError(cycles, providerMapMirror);
+ }
+
+ // Try again after hopefully having removed all cycles
+ try {
+ reversedTopographicSort = Lists.reverse(TopographicGraphSorter.sortGraph(loadOrderGraph));
+ } catch (TopographicGraphSorter.GraphCycleException e) {
+ throw new PluginGraphCycleException(cycles);
+ }
+ }
+
+ GraphDependencyContext graphDependencyContext = new GraphDependencyContext(dependencyGraph);
@ -2544,6 +2570,45 @@ index 0000000000000000000000000000000000000000..fa7bbfeff53d5cab3d96a5f1181b610f
+ return loadedPlugins;
+ }
+
+ private void logCycleError(List<List<String>> cycles, Map<String, PluginProvider<?>> providerMapMirror) {
+ LOGGER.error("=================================");
+ LOGGER.error("Circular plugin loading detected:");
+ for (int i = 0; i < cycles.size(); i++) {
+ List<String> cycle = cycles.get(i);
+ LOGGER.error("{}) {} -> {}", i + 1, String.join(" -> ", cycle), cycle.get(0));
+ for (String pluginName : cycle) {
+ PluginProvider<?> pluginProvider = providerMapMirror.get(pluginName);
+ if (pluginProvider == null) {
+ return;
+ }
+
+ logPluginInfo(pluginProvider.getMeta());
+ }
+ }
+
+ LOGGER.error("Please report this to the plugin authors of the first plugin of each loop or join the PaperMC Discord server for further help.");
+ LOGGER.error("=================================");
+ }
+
+ private void logPluginInfo(PluginMeta meta) {
+ if (!meta.getLoadBeforePlugins().isEmpty()) {
+ LOGGER.error(" {} loadbefore: {}", meta.getName(), meta.getLoadBeforePlugins());
+ }
+
+ if (meta instanceof PaperPluginMeta paperPluginMeta) {
+ if (!paperPluginMeta.getLoadAfterPlugins().isEmpty()) {
+ LOGGER.error(" {} loadafter: {}", meta.getName(), paperPluginMeta.getLoadAfterPlugins());
+ }
+ } else {
+ List<String> dependencies = new ArrayList<>();
+ dependencies.addAll(meta.getPluginDependencies());
+ dependencies.addAll(meta.getPluginSoftDependencies());
+ if (!dependencies.isEmpty()) {
+ LOGGER.error(" {} depend/softdepend: {}", meta.getName(), dependencies);
+ }
+ }
+ }
+
+ private static class PluginProviderEntry<T> {
+
+ private final PluginProvider<T> provider;
@ -2632,28 +2697,27 @@ index 0000000000000000000000000000000000000000..dee83e821dcc9baf3a3e5ca8325b03ed
+}
diff --git a/src/main/java/io/papermc/paper/plugin/entrypoint/strategy/TopographicGraphSorter.java b/src/main/java/io/papermc/paper/plugin/entrypoint/strategy/TopographicGraphSorter.java
new file mode 100644
index 0000000000000000000000000000000000000000..0720af0d48b39ca46e7d3aba08d7b359ed053461
index 0000000000000000000000000000000000000000..52a110044611c8a0ace6d49549e8acc16cbbe83d
--- /dev/null
+++ b/src/main/java/io/papermc/paper/plugin/entrypoint/strategy/TopographicGraphSorter.java
@@ -0,0 +1,61 @@
@@ -0,0 +1,58 @@
+package io.papermc.paper.plugin.entrypoint.strategy;
+
+import com.google.common.graph.Graph;
+
+import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public class TopographicGraphSorter {
+public final class TopographicGraphSorter {
+
+ // Topographically sort dependencies
+ public static <N> List<N> sortGraph(Graph<N> graph) throws PluginGraphCycleException {
+ List<N> sorted = new ArrayList<>();
+ Deque<N> roots = new ArrayDeque<>();
+ Map<N, Integer> nonRoots = new HashMap<>();
+ Object2IntMap<N> nonRoots = new Object2IntOpenHashMap<>();
+
+ for (N node : graph.nodes()) {
+ // Is a node being referred to by any other nodes?
@ -2668,15 +2732,13 @@ index 0000000000000000000000000000000000000000..0720af0d48b39ca46e7d3aba08d7b359
+ }
+
+ // Pick from nodes that aren't referred to anywhere else
+ while (!roots.isEmpty()) {
+ N next = roots.remove();
+
+ N next;
+ while ((next = roots.poll()) != null) {
+ for (N successor : graph.successors(next)) {
+ // Traverse through, moving down a degree
+ int newInDegree = nonRoots.get(successor) - 1;
+ int newInDegree = nonRoots.removeInt(successor) - 1;
+
+ if (newInDegree == 0) {
+ nonRoots.remove(successor);
+ roots.add(successor);
+ } else {
+ nonRoots.put(successor, newInDegree);
@ -2693,7 +2755,7 @@ index 0000000000000000000000000000000000000000..0720af0d48b39ca46e7d3aba08d7b359
+ return sorted;
+ }
+
+ public static class GraphCycleException extends RuntimeException {
+ public static final class GraphCycleException extends RuntimeException {
+
+ }
+}
@ -6190,7 +6252,7 @@ index 0000000000000000000000000000000000000000..fbe76a678f45bd3c55f25f2b6a4366ef
+}
diff --git a/src/main/java/io/papermc/paper/plugin/storage/SimpleProviderStorage.java b/src/main/java/io/papermc/paper/plugin/storage/SimpleProviderStorage.java
new file mode 100644
index 0000000000000000000000000000000000000000..272b1d13a4925c92f39e02fc8360521a6b284727
index 0000000000000000000000000000000000000000..a7085dbf2c5ace9b6887d024c0785fa529e0f1da
--- /dev/null
+++ b/src/main/java/io/papermc/paper/plugin/storage/SimpleProviderStorage.java
@@ -0,0 +1,85 @@
@ -6257,7 +6319,7 @@ index 0000000000000000000000000000000000000000..272b1d13a4925c92f39e02fc8360521a
+ LOGGER.error("Circular plugin loading detected!");
+ LOGGER.error("Circular load order:");
+ for (String logMessage : logMessages) {
+ LOGGER.error(" " + logMessage);
+ LOGGER.error(" {}", logMessage);
+ }
+ LOGGER.error("Please report this to the plugin authors of the first plugin of each loop or join the PaperMC Discord server for further help.");
+ LOGGER.error("If you would like to still load these plugins, acknowledging that there may be unexpected plugin loading issues, run the server with -Dpaper.useLegacyPluginLoading=true");