From 089a793ba01f0c20745dd933cbc9550784444360 Mon Sep 17 00:00:00 2001 From: Max Weber Date: Fri, 31 Jan 2020 18:10:28 -0700 Subject: [PATCH] PluginManager: require start/stop to be invoked on the EDT The current way of invoking it on the executor would let the client have a start and a stop for a plugin sitting in the executor's queue, making it toggle endlessly. This only was because we were using invokeAndWait in startPlugin, which can't be invoked on the EDT because it would deadlock with itself. --- .../ExternalPluginManager.java | 49 ++++++++++-- .../client/plugins/PluginManager.java | 74 +++++++++---------- .../plugins/config/PluginListPanel.java | 42 +++++------ .../client/plugins/gpu/GpuPlugin.java | 20 +++-- 4 files changed, 106 insertions(+), 79 deletions(-) diff --git a/runelite-client/src/main/java/net/runelite/client/externalplugins/ExternalPluginManager.java b/runelite-client/src/main/java/net/runelite/client/externalplugins/ExternalPluginManager.java index 531f443cf9..2a3c3a828c 100644 --- a/runelite-client/src/main/java/net/runelite/client/externalplugins/ExternalPluginManager.java +++ b/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 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); } 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 3cbcc57385..ed814d27cb 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 @@ -132,8 +132,9 @@ public class PluginManager private void refreshPlugins() { loadDefaultPluginConfiguration(null); - getPlugins() - .forEach(plugin -> executor.submit(() -> + SwingUtilities.invokeLater(() -> + { + 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); - plugins.remove(plugin); + throw new RuntimeException(e); } 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 - SwingUtilities.invokeAndWait(() -> - { - try - { - plugin.startUp(); - } - catch (Exception ex) - { - throw new RuntimeException(ex); - } - }); + plugin.startUp(); 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); - eventBus.unregister(plugin); - - // plugins always stop in the event thread - SwingUtilities.invokeAndWait(() -> - { - try - { - plugin.shutDown(); - } - catch (Exception ex) - { - throw new RuntimeException(ex); - } - }); + plugin.shutDown(); 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 * @return diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/config/PluginListPanel.java b/runelite-client/src/main/java/net/runelite/client/plugins/config/PluginListPanel.java index b7b08b780b..2e96a4dae9 100644 --- a/runelite-client/src/main/java/net/runelite/client/plugins/config/PluginListPanel.java +++ b/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 - { - pluginManager.startPlugin(plugin); - } - catch (PluginInstantiationException ex) - { - log.warn("Error when starting plugin {}", plugin.getClass().getSimpleName(), ex); - } - }); + try + { + pluginManager.startPlugin(plugin); + } + catch (PluginInstantiationException 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 - { - pluginManager.stopPlugin(plugin); - } - catch (PluginInstantiationException ex) - { - log.warn("Error when stopping plugin {}", plugin.getClass().getSimpleName(), ex); - } - }); + try + { + pluginManager.stopPlugin(plugin); + } + catch (PluginInstantiationException ex) + { + log.warn("Error when stopping plugin {}", plugin.getClass().getSimpleName(), ex); + } } private List getPinnedPluginNames() diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/gpu/GpuPlugin.java b/runelite-client/src/main/java/net/runelite/client/plugins/gpu/GpuPlugin.java index 66e0977b32..e4a0233c0f 100644 --- a/runelite-client/src/main/java/net/runelite/client/plugins/gpu/GpuPlugin.java +++ b/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); - pluginManager.stopPlugin(this); - } - catch (PluginInstantiationException ex) - { - log.error("error stopping plugin", ex); - } + try + { + pluginManager.setPluginEnabled(this, false); + pluginManager.stopPlugin(this); + } + catch (PluginInstantiationException ex) + { + log.error("error stopping plugin", ex); + } + }); shutDown(); }