From eab0a0a28e2accb29bc645e80878f1101c934923 Mon Sep 17 00:00:00 2001 From: Nassim Jahnke Date: Mon, 27 Feb 2023 12:09:10 +0100 Subject: [PATCH] Don't log or die on cyclic dependencies of Spigot plugins --- patches/server/Paper-Plugins.patch | 96 ++++++++++++++++++++++++------ 1 file changed, 79 insertions(+), 17 deletions(-) diff --git a/patches/server/Paper-Plugins.patch b/patches/server/Paper-Plugins.patch index c9e887f676..d7b8d03b18 100644 --- a/patches/server/Paper-Plugins.patch +++ b/patches/server/Paper-Plugins.patch @@ -1811,6 +1811,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 +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..00000000000000000000000000000000 + + // The main state of the algorithm. + private Consumer> cycleConsumer = null; ++ private BiConsumer cycleVertexSuccessorConsumer = null; // Paper + private V[] iToV = null; + private Map vToI = null; + private Set blocked = null; @@ -1865,10 +1867,10 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + * + * @return The list of all simple cycles. Possibly empty but never null. + */ -+ public List> findSimpleCycles() ++ public List> findAndRemoveSimpleCycles() + { + List> result = new ArrayList<>(); -+ findSimpleCycles(result::add); ++ findSimpleCycles(result::add, (v, s) -> ((MutableGraph) graph).removeEdge(v, s)); // Paper + return result; + } + @@ -1877,11 +1879,12 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + * + * @param consumer Consumer that will be called with each cycle found. + */ -+ public void findSimpleCycles(Consumer> consumer) ++ public void findSimpleCycles(Consumer> consumer, BiConsumer vertexSuccessorConsumer) // Paper + { + if (graph == null) { + throw new IllegalArgumentException("Null graph."); + } ++ cycleVertexSuccessorConsumer = vertexSuccessorConsumer; // Paper + initState(consumer); + + int startIndex = 0; @@ -2032,7 +2035,8 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + List 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; @@ -2418,6 +2422,10 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 +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..00000000000000000000000000000000 + try { + reversedTopographicSort = Lists.reverse(TopographicGraphSorter.sortGraph(loadOrderGraph)); + } catch (TopographicGraphSorter.GraphCycleException exception) { -+ throw new PluginGraphCycleException(new JohnsonSimpleCycles<>(loadOrderGraph).findSimpleCycles()); ++ List> 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 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..00000000000000000000000000000000 + return loadedPlugins; + } + ++ private void logCycleError(List> cycles, Map> providerMapMirror) { ++ LOGGER.error("================================="); ++ LOGGER.error("Circular plugin loading detected:"); ++ for (int i = 0; i < cycles.size(); i++) { ++ List 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 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 { + + private final PluginProvider provider; @@ -2639,21 +2704,20 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 +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 List sortGraph(Graph graph) throws PluginGraphCycleException { + List sorted = new ArrayList<>(); + Deque roots = new ArrayDeque<>(); -+ Map nonRoots = new HashMap<>(); ++ Object2IntMap nonRoots = new Object2IntOpenHashMap<>(); + + for (N node : graph.nodes()) { + // Is a node being referred to by any other nodes? @@ -2668,15 +2732,13 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + } + + // 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..00000000000000000000000000000000 + return sorted; + } + -+ public static class GraphCycleException extends RuntimeException { ++ public static final class GraphCycleException extends RuntimeException { + + } +} @@ -6257,7 +6319,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + 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");