From e17baf1aee342282ea73cbad40844f47568eefc8 Mon Sep 17 00:00:00 2001
From: LootBagger <91767458+LootBagger@users.noreply.github.com>
Date: Sat, 5 Feb 2022 10:38:30 -0800
Subject: [PATCH] plugin manager: fix plugins with multiple dependencies

If one plugin has multiple dependencies this would throw a concurrent
modification exception due to iterating the successors after removing
the edge

Fill out javadoc for topologicalSort

Co-authored-by: Adam <Adam@sigterm.info>
---
 .../client/plugins/PluginManager.java         | 13 +++++++----
 .../client/plugins/PluginManagerTest.java     | 23 +++++++++++++++++++
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/PluginManager.java b/runelite-client/src/main/java/net/runelite/client/plugins/PluginManager.java
index c9c88b669a..ccbf5e61c7 100644
--- a/runelite-client/src/main/java/net/runelite/client/plugins/PluginManager.java
+++ b/runelite-client/src/main/java/net/runelite/client/plugins/PluginManager.java
@@ -631,11 +631,14 @@ public class PluginManager
 	/**
 	 * Topologically sort a graph. Uses Kahn's algorithm.
 	 *
-	 * @param graph
-	 * @param <T>
-	 * @return
+	 * @param graph - A directed graph
+	 * @param <T>   - The type of the item contained in the nodes of the graph
+	 * @return - A topologically sorted list corresponding to graph.
+	 * <p>
+	 * Multiple invocations with the same arguments may return lists that are not equal.
 	 */
-	private <T> List<T> topologicalSort(Graph<T> graph)
+	@VisibleForTesting
+	static <T> List<T> topologicalSort(Graph<T> graph)
 	{
 		MutableGraph<T> graphCopy = Graphs.copyOf(graph);
 		List<T> l = new ArrayList<>();
@@ -650,7 +653,7 @@ public class PluginManager
 
 			l.add(n);
 
-			for (T m : graphCopy.successors(n))
+			for (T m : new HashSet<>(graphCopy.successors(n)))
 			{
 				graphCopy.removeEdge(n, m);
 				if (graphCopy.inDegree(m) == 0)
diff --git a/runelite-client/src/test/java/net/runelite/client/plugins/PluginManagerTest.java b/runelite-client/src/test/java/net/runelite/client/plugins/PluginManagerTest.java
index 983bf613f2..3166552b6a 100644
--- a/runelite-client/src/test/java/net/runelite/client/plugins/PluginManagerTest.java
+++ b/runelite-client/src/test/java/net/runelite/client/plugins/PluginManagerTest.java
@@ -24,6 +24,8 @@
  */
 package net.runelite.client.plugins;
 
+import com.google.common.graph.GraphBuilder;
+import com.google.common.graph.MutableGraph;
 import com.google.common.reflect.ClassPath;
 import com.google.common.reflect.ClassPath.ClassInfo;
 import com.google.inject.Guice;
@@ -40,6 +42,7 @@ import java.io.PrintWriter;
 import java.lang.reflect.Method;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Objects;
 import java.util.Set;
 import net.runelite.api.Client;
@@ -51,6 +54,7 @@ import net.runelite.client.eventbus.EventBus;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -208,4 +212,23 @@ public class PluginManagerTest
 		}
 	}
 
+	@Test
+	public void testTopologicalSort()
+	{
+		MutableGraph<Integer> graph = GraphBuilder
+			.directed()
+			.build();
+
+		graph.addNode(1);
+		graph.addNode(2);
+		graph.addNode(3);
+
+		graph.putEdge(1, 2);
+		graph.putEdge(1, 3);
+
+		List<Integer> sorted = PluginManager.topologicalSort(graph);
+
+		assertTrue(sorted.indexOf(1) < sorted.indexOf(2));
+		assertTrue(sorted.indexOf(1) < sorted.indexOf(3));
+	}
 }