Merge pull request #10707 from abextm/startplugin-edt

PluginManager: require start/stop to be invoked on the EDT

runelite-client/src/main/java/net/runelite/client/externalplugins/ExternalPluginManager.java

@@ -33,6 +33,7 @@ import com.google.common.hash.HashingInputStream;
 import com.google.common.io.Files;
 import java.io.File;
 import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -44,6 +45,7 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.function.Function;
 import javax.inject.Inject;
 import javax.inject.Singleton;
+import javax.swing.SwingUtilities;
 import lombok.extern.slf4j.Slf4j;
 import net.runelite.client.RuneLite;
 import net.runelite.client.RuneLiteProperties;
@@ -242,9 +244,19 @@ public class ExternalPluginManager
 				log.info("Stopping external plugin \"{}\"", p.getClass());
 				try
 				{
-					pluginManager.stopPlugin(p);
+					SwingUtilities.invokeAndWait(() ->
+					{
+						try
+						{
+							pluginManager.stopPlugin(p);
+						}
+						catch (Exception e)
+						{
+							throw new RuntimeException(e);
+						}
+					});
 				}
-				catch (PluginInstantiationException e)
+				catch (InterruptedException | InvocationTargetException e)
 				{
 					log.warn("Unable to stop external plugin \"{}\"", p.getClass().getName(), e);
 				}
@@ -272,15 +284,25 @@ public class ExternalPluginManager
 						clazzes.add(cl.loadClass(className));
 					}
 
-					newPlugins = pluginManager.loadPlugins(clazzes, null);
+					List<Plugin> newPlugins2 = newPlugins = pluginManager.loadPlugins(clazzes, null);
 					if (!startup)
 					{
 						pluginManager.loadDefaultPluginConfiguration(newPlugins);
 
-						for (Plugin p : newPlugins)
+						SwingUtilities.invokeAndWait(() ->
 						{
-							pluginManager.startPlugin(p);
+							try
-						}
+							{
+								for (Plugin p : newPlugins2)
+								{
+									pluginManager.startPlugin(p);
+								}
+							}
+							catch (PluginInstantiationException e)
+							{
+								throw new RuntimeException(e);
+							}
+						});
 					}
 				}
 				catch (Exception e)
@@ -292,10 +314,21 @@ public class ExternalPluginManager
 						{
 							try
 							{
-								pluginManager.stopPlugin(p);
+								SwingUtilities.invokeAndWait(() ->
+								{
+									try
+									{
+										pluginManager.stopPlugin(p);
+									}
+									catch (Exception e2)
+									{
+										throw new RuntimeException(e2);
+									}
+								});
 							}
-							catch (Exception inner)
+							catch (InterruptedException | InvocationTargetException e2)
 							{
+								log.info("Unable to fully stop plugin \"{}\"", manifest.getInternalName(), e2);
 							}
 							pluginManager.remove(p);
 						}

runelite-client/src/main/java/net/runelite/client/plugins/PluginManager.java

@@ -132,8 +132,9 @@ public class PluginManager
 	private void refreshPlugins()
 	{
 		loadDefaultPluginConfiguration(null);
-		getPlugins()
+		SwingUtilities.invokeLater(() ->
-			.forEach(plugin -> executor.submit(() ->
+		{
+			for (Plugin plugin : getPlugins())
 			{
 				try
 				{
@@ -153,7 +154,8 @@ public class PluginManager
 				{
 					log.warn("Error during starting/stopping plugin {}", plugin.getClass().getSimpleName(), e);
 				}
-			}));
+			}
+		});
 	}
 
 	public Config getPluginConfigProxy(Plugin plugin)
@@ -215,12 +217,22 @@ public class PluginManager
 		{
 			try
 			{
-				startPlugin(plugin);
+				SwingUtilities.invokeAndWait(() ->
+				{
+					try
+					{
+						startPlugin(plugin);
+					}
+					catch (PluginInstantiationException ex)
+					{
+						log.warn("Unable to start plugin {}", plugin.getClass().getSimpleName(), ex);
+						plugins.remove(plugin);
+					}
+				});
 			}
-			catch (PluginInstantiationException ex)
+			catch (InterruptedException | InvocationTargetException e)
 			{
-				log.warn("Unable to start plugin {}", plugin.getClass().getSimpleName(), ex);
+				throw new RuntimeException(e);
-				plugins.remove(plugin);
 			}
 
 			loaded++;
@@ -325,8 +337,11 @@ public class PluginManager
 		return newPlugins;
 	}
 
-	public synchronized boolean startPlugin(Plugin plugin) throws PluginInstantiationException
+	public boolean startPlugin(Plugin plugin) throws PluginInstantiationException
 	{
+		// plugins always start in the EDT
+		assert SwingUtilities.isEventDispatchThread();
+
 		if (activePlugins.contains(plugin) || !isPluginEnabled(plugin))
 		{
 			return false;
@@ -336,18 +351,7 @@ public class PluginManager
 
 		try
 		{
-			// plugins always start in the event thread
+			plugin.startUp();
-			SwingUtilities.invokeAndWait(() ->
-			{
-				try
-				{
-					plugin.startUp();
-				}
-				catch (Exception ex)
-				{
-					throw new RuntimeException(ex);
-				}
-			});
 
 			log.debug("Plugin {} is now running", plugin.getClass().getSimpleName());
 			if (!isOutdated && sceneTileManager != null)
@@ -363,7 +367,7 @@ public class PluginManager
 			schedule(plugin);
 			eventBus.post(new PluginChanged(plugin, true));
 		}
-		catch (InterruptedException | InvocationTargetException | IllegalArgumentException ex)
+		catch (Exception ex)
 		{
 			throw new PluginInstantiationException(ex);
 		}
@@ -371,36 +375,27 @@ public class PluginManager
 		return true;
 	}
 
-	public synchronized boolean stopPlugin(Plugin plugin) throws PluginInstantiationException
+	public boolean stopPlugin(Plugin plugin) throws PluginInstantiationException
 	{
+		// plugins always stop in the EDT
+		assert SwingUtilities.isEventDispatchThread();
+
 		if (!activePlugins.remove(plugin))
 		{
 			return false;
 		}
 
+		unschedule(plugin);
+		eventBus.unregister(plugin);
+
 		try
 		{
-			unschedule(plugin);
+			plugin.shutDown();
-			eventBus.unregister(plugin);
-
-			// plugins always stop in the event thread
-			SwingUtilities.invokeAndWait(() ->
-			{
-				try
-				{
-					plugin.shutDown();
-				}
-				catch (Exception ex)
-				{
-					throw new RuntimeException(ex);
-				}
-			});
 
 			log.debug("Plugin {} is now stopped", plugin.getClass().getSimpleName());
 			eventBus.post(new PluginChanged(plugin, false));
-
 		}
-		catch (InterruptedException | InvocationTargetException ex)
+		catch (Exception ex)
 		{
 			throw new PluginInstantiationException(ex);
 		}
@@ -555,6 +550,7 @@ public class PluginManager
 
 	/**
 	 * Topologically sort a graph. Uses Kahn's algorithm.
+	 *
 	 * @param graph
 	 * @param <T>
 	 * @return

runelite-client/src/main/java/net/runelite/client/plugins/config/PluginListPanel.java

@@ -311,36 +311,30 @@ class PluginListPanel extends PluginPanel
 
 	void startPlugin(Plugin plugin)
 	{
-		executorService.submit(() ->
+		pluginManager.setPluginEnabled(plugin, true);
-		{
-			pluginManager.setPluginEnabled(plugin, true);
 
-			try
+		try
-			{
+		{
-				pluginManager.startPlugin(plugin);
+			pluginManager.startPlugin(plugin);
-			}
+		}
-			catch (PluginInstantiationException ex)
+		catch (PluginInstantiationException ex)
-			{
+		{
-				log.warn("Error when starting plugin {}", plugin.getClass().getSimpleName(), ex);
+			log.warn("Error when starting plugin {}", plugin.getClass().getSimpleName(), ex);
-			}
+		}
-		});
 	}
 
 	void stopPlugin(Plugin plugin)
 	{
-		executorService.submit(() ->
+		pluginManager.setPluginEnabled(plugin, false);
-		{
-			pluginManager.setPluginEnabled(plugin, false);
 
-			try
+		try
-			{
+		{
-				pluginManager.stopPlugin(plugin);
+			pluginManager.stopPlugin(plugin);
-			}
+		}
-			catch (PluginInstantiationException ex)
+		catch (PluginInstantiationException ex)
-			{
+		{
-				log.warn("Error when stopping plugin {}", plugin.getClass().getSimpleName(), ex);
+			log.warn("Error when stopping plugin {}", plugin.getClass().getSimpleName(), ex);
-			}
+		}
-		});
 	}
 
 	private List<String> getPinnedPluginNames()

runelite-client/src/main/java/net/runelite/client/plugins/gpu/GpuPlugin.java

@@ -48,6 +48,7 @@ import java.nio.FloatBuffer;
 import java.nio.IntBuffer;
 import java.util.function.Function;
 import javax.inject.Inject;
+import javax.swing.SwingUtilities;
 import jogamp.nativewindow.SurfaceScaleUtils;
 import jogamp.nativewindow.jawt.x11.X11JAWTWindow;
 import jogamp.newt.awt.NewtFactoryAWT;
@@ -334,15 +335,18 @@ public class GpuPlugin extends Plugin implements DrawCallbacks
 			{
 				log.error("Error starting GPU plugin", e);
 
-				try
+				SwingUtilities.invokeLater(() ->
 				{
-					pluginManager.setPluginEnabled(this, false);
+					try
-					pluginManager.stopPlugin(this);
+					{
-				}
+						pluginManager.setPluginEnabled(this, false);
-				catch (PluginInstantiationException ex)
+						pluginManager.stopPlugin(this);
-				{
+					}
-					log.error("error stopping plugin", ex);
+					catch (PluginInstantiationException ex)
-				}
+					{
+						log.error("error stopping plugin", ex);
+					}
+				});
 
 				shutDown();
 			}