diff --git a/src/main/java/org/bukkit/fillr/Fillr.java b/src/main/java/org/bukkit/fillr/Fillr.java index 9f650c0044..93ce02611d 100644 --- a/src/main/java/org/bukkit/fillr/Fillr.java +++ b/src/main/java/org/bukkit/fillr/Fillr.java @@ -14,17 +14,16 @@ public class Fillr extends JavaPlugin { private static final Logger logger = Logger.getLogger(Fillr.class.getName()); - public void onEnable() { + public boolean onEnable() { if (getServer().getPluginManager() instanceof SimplePluginManager) { registerEvents(); + return true; } else { logger.log(Level.WARNING, "Fillr only works with SimplePluginManager"); + return false; } } - public void onDisable() { - } - private void registerEvents() { FillrListener listener = new FillrListener(getServer()); registerEvent(Event.Type.PLAYER_COMMAND, Event.Priority.Normal, listener); diff --git a/src/main/java/org/bukkit/plugin/InvalidPluginException.java b/src/main/java/org/bukkit/plugin/InvalidPluginException.java index 0f7bfdfe66..18b955868f 100644 --- a/src/main/java/org/bukkit/plugin/InvalidPluginException.java +++ b/src/main/java/org/bukkit/plugin/InvalidPluginException.java @@ -9,30 +9,45 @@ public class InvalidPluginException extends Exception { private final Plugin plugin; /** - * Constructs a new InvalidPluginException based on the given Exception + * Constructs a new InvalidPluginException for the given Plugin and Exception + * + * Specifying a plugin using this constructor allows the PluginManager to + * do cleanup of any resources the plugin may have already registered. * * @param throwable Exception that triggered this Exception + * @param plugin Plugin that caused the exception */ - public InvalidPluginException(Throwable throwable) { + public InvalidPluginException(Throwable throwable, Plugin plugin) { cause = throwable; - plugin = null; + this.plugin = plugin; } /** - * Constructs a new InvalidPluginException for the given Plugin and Exception + * Constructs a new InvalidPluginException for the given Plugin * - * Even if a plugin failed to load, a Plugin instance may already have been - * created, and resources acquired in name of it. Throwing this exception - * using this constructor allows a manager to release these resources. + * Specifying a plugin using this constructor allows the PluginManager to + * do cleanup of any resources the plugin may have already registered. * - * @param throwable Exception that triggered this Exception * @param plugin Plugin that caused the exception */ - public InvalidPluginException(Throwable throwable, Plugin plugin) { - cause = throwable; + public InvalidPluginException(Plugin plugin) { + cause = null; this.plugin = plugin; } + /** + * Constructs a new InvalidPluginException based on the given Exception + * + * The constructors that accept a Plugin instance are preferred to this + * constructor. Only use this constructor if no cleanup is necessary. + * + * @param throwable Exception that triggered this Exception + */ + public InvalidPluginException(Throwable throwable) { + cause = throwable; + plugin = null; + } + /** * Constructs a new InvalidPluginException */ diff --git a/src/main/java/org/bukkit/plugin/PluginInhibitException.java b/src/main/java/org/bukkit/plugin/PluginInhibitException.java new file mode 100644 index 0000000000..4896f73641 --- /dev/null +++ b/src/main/java/org/bukkit/plugin/PluginInhibitException.java @@ -0,0 +1,43 @@ +package org.bukkit.plugin; + +/** + * Thrown when a plugin requests to inhibit loading or unloading. + */ +public final class PluginInhibitException extends Exception { + private static final long serialVersionUID = 3857877689996365765L; + private final Plugin plugin; + + /** + * Constructor with associated plugin. + * + * Specifying a plugin using this constructor allows the PluginManager to + * do cleanup of any resources the plugin may have already registered. + * + * @param plugin The plugin that requested inhibition + */ + public PluginInhibitException(Plugin plugin) { + super(); + this.plugin = plugin; + } + + /** + * Default constructor. + * + * The preferred constructor is + * {@link PluginInhibitException#PluginInhibitException(Plugin)}. + * This constructor is only to be used if no cleanup is necessary. + */ + public PluginInhibitException() { + super(); + this.plugin = null; + } + + /** + * Get the plugin that requested inhibition. + * + * @return A Plugin instance + */ + public Plugin getPlugin() { + return plugin; + } +} diff --git a/src/main/java/org/bukkit/plugin/PluginLoader.java b/src/main/java/org/bukkit/plugin/PluginLoader.java index 30b0a88355..8cc45a9578 100644 --- a/src/main/java/org/bukkit/plugin/PluginLoader.java +++ b/src/main/java/org/bukkit/plugin/PluginLoader.java @@ -53,8 +53,9 @@ public interface PluginLoader { * @param description The plugin description object identifying the plugin * @return The plugin that was loaded * @throws InvalidPluginException Thrown when the specified file is not a plugin + * @throws PluginInhibitException Thrown by the plugin to cancel enabling */ - public Plugin enablePlugin(PluginDescription description) throws InvalidPluginException; + public Plugin enablePlugin(PluginDescription description) throws InvalidPluginException, PluginInhibitException; /** * Called by PluginManager to disable a plugin @@ -70,6 +71,7 @@ public interface PluginLoader { * instead. * * @param plugin The plugin to unload + * @throws PluginInhibitException Thrown by the plugin to cancel disabling */ - public void disablePlugin(Plugin plugin); + public void disablePlugin(Plugin plugin) throws PluginInhibitException; } diff --git a/src/main/java/org/bukkit/plugin/SimplePluginManager.java b/src/main/java/org/bukkit/plugin/SimplePluginManager.java index 7643af259d..9a7ff068f2 100644 --- a/src/main/java/org/bukkit/plugin/SimplePluginManager.java +++ b/src/main/java/org/bukkit/plugin/SimplePluginManager.java @@ -133,7 +133,6 @@ public Plugin visit(PluginDescription description) throws Exception { if (plugin == null) { PluginLoader loader = description.getLoader(); plugin = loader.enablePlugin(description); - plugins.put(description.getName(), plugin); callEvent(new PluginEvent(Event.Type.PLUGIN_ENABLE, plugin)); } @@ -158,14 +157,22 @@ else if (ex instanceof MissingDependencyException) { } else { Throwable inner = ex.getCause(); - server.getLogger().log(Level.SEVERE, "Could not load " + description.getName() + ": " + inner.getMessage(), inner); + Plugin plugin = null; - if (inner instanceof InvalidPluginException) { - Plugin instance = ((InvalidPluginException)inner).getPlugin(); - if (instance != null) { - releasePluginResources(description.getName(), instance); + if (inner instanceof PluginInhibitException) { + server.getLogger().log(Level.INFO, "Plugin " + description.getName() + " inhibited loading."); + plugin = ((PluginInhibitException)inner).getPlugin(); + } + else { + server.getLogger().log(Level.SEVERE, "Could not load " + description.getName() + ": " + inner.getMessage(), inner); + if (inner instanceof InvalidPluginException) { + plugin = ((InvalidPluginException)inner).getPlugin(); } } + + if (plugin != null) { + releasePluginResources(description.getName(), plugin); + } } } @@ -264,7 +271,12 @@ public void disablePlugin(final Plugin plugin) { server.getLogger().log(Level.SEVERE, "Could not unload " + description.getName() + ": " + ex.getMessage()); } catch (GraphIterationAborted ex) { Throwable inner = ex.getCause(); - server.getLogger().log(Level.SEVERE, "Could not unload " + description.getName() + ": " + inner.getMessage(), inner); + if (inner instanceof PluginInhibitException) { + server.getLogger().log(Level.INFO, "Plugin " + description.getName() + " inhibited unloading."); + } + else { + server.getLogger().log(Level.SEVERE, "Could not unload " + description.getName() + ": " + inner.getMessage(), inner); + } } } diff --git a/src/main/java/org/bukkit/plugin/java/JavaPlugin.java b/src/main/java/org/bukkit/plugin/java/JavaPlugin.java index bb712c1142..c78021eb16 100644 --- a/src/main/java/org/bukkit/plugin/java/JavaPlugin.java +++ b/src/main/java/org/bukkit/plugin/java/JavaPlugin.java @@ -116,9 +116,12 @@ public PluginDescription getDescription() { * Called when this plugin is enabled * * This is the place to do initialization, such as installing listeners. + * + * @return True on success, false to abort enabling. */ - public void onEnable() { + public boolean onEnable() { // default implementation: do nothing! + return true; } /** @@ -127,9 +130,12 @@ public void onEnable() { * The PluginManager sees to cleaning up commands, listeners, loaders and * scheduled tasks, but any other resources held by the plugin should be * cleaned up here. + * + * @return True on success, false to abort enabling. */ - public void onDisable() { + public boolean onDisable() { // default implementation: do nothing! + return true; } /** diff --git a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java index fbad3f5264..d708cbf833 100644 --- a/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java +++ b/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java @@ -6,6 +6,7 @@ import java.io.IOException; import java.io.InputStream; import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.List; import java.util.jar.JarEntry; @@ -140,9 +141,9 @@ private PluginDescription readSystemPluginDescription(File file) throws InvalidD /** * {@inheritDoc} */ - public Plugin enablePlugin(PluginDescription abstractDescription) throws InvalidPluginException { + public Plugin enablePlugin(PluginDescription abstractDescription) throws InvalidPluginException, PluginInhibitException { JavaPluginDescription description = (JavaPluginDescription)abstractDescription; - JavaPlugin plugin = null; + Plugin retval = null; // The reflection voodoo, where we find the class and constructor. PluginClassLoader cloader = description.getClassLoader(); @@ -155,57 +156,88 @@ public Plugin enablePlugin(PluginDescription abstractDescription) throws Invalid throw new InvalidPluginException(ex); } - // Instantiate the plugin. + // The ClassLoader should be available during and after enabling, + // but should be cleaned up if things don't work out. pluginClassLoaders.add(cloader); try { + // Instantiate the plugin. + JavaPlugin plugin; try { - Constructor constructor = pluginClass.getConstructor(Server.class, JavaPluginDescription.class); - plugin = constructor.newInstance(server, description); - } catch (NoSuchMethodException ex) { - Constructor constructor = pluginClass.getConstructor(); - plugin = constructor.newInstance(); + try { + Constructor constructor = pluginClass.getConstructor(Server.class, JavaPluginDescription.class); + plugin = constructor.newInstance(server, description); + } catch (NoSuchMethodException ex) { + Constructor constructor = pluginClass.getConstructor(); + plugin = constructor.newInstance(); + } + } + catch (NoSuchMethodException ex) { + throw new InvalidPluginException(ex); + } + catch (IllegalArgumentException ex) { + throw new InvalidPluginException(ex); + } + catch (InstantiationException ex) { + throw new InvalidPluginException(ex); + } + catch (IllegalAccessException ex) { + throw new InvalidPluginException(ex); + } + catch (InvocationTargetException ex) { + throw new InvalidPluginException(ex.getCause()); } - } - catch (Throwable ex) { - pluginClassLoaders.remove(cloader); - throw new InvalidPluginException(ex, plugin); - } - // Set up private fields. - plugin.initialize(server, description); + // Set up private fields. + plugin.initialize(server, description); - // Install commands from plugin.yml. - List pluginCommands = description.buildCommands(plugin); - if (!pluginCommands.isEmpty()) { - final CommandMap commandMap = server.getPluginManager().getCommandMap(); - commandMap.registerAll(plugin.getDescription().getName(), pluginCommands); - } + // Install commands from plugin.yml. + List pluginCommands = description.buildCommands(plugin); + if (!pluginCommands.isEmpty()) { + final CommandMap commandMap = server.getPluginManager().getCommandMap(); + commandMap.registerAll(plugin.getDescription().getName(), pluginCommands); + } - // Call the onEnable method. - try { - plugin.onEnable(); + // Call the onEnable method. + boolean inhibit; + try { + inhibit = !plugin.onEnable(); + } + catch (Throwable ex) { + throw new InvalidPluginException(ex, plugin); + } + if (inhibit) { + throw new PluginInhibitException(plugin); + } + + retval = plugin; } - catch (Throwable ex) { - pluginClassLoaders.remove(cloader); - throw new InvalidPluginException(ex, plugin); + finally { + if (retval == null) { + pluginClassLoaders.remove(cloader); + } } - return plugin; + return retval; } /** * {@inheritDoc} */ - public void disablePlugin(Plugin plugin) { + public void disablePlugin(Plugin plugin) throws PluginInhibitException { JavaPlugin jPlugin = (JavaPlugin)plugin; JavaPluginDescription description = (JavaPluginDescription)plugin.getDescription(); + boolean inhibit = false; try { - jPlugin.onDisable(); + inhibit = !jPlugin.onDisable(); } catch (Throwable ex) { + // We log, but keep on unloading. log.log(Level.SEVERE, "Plugin " + description.getName() + " threw an exception while disabling.", ex); } + if (inhibit) { + throw new PluginInhibitException(plugin); + } pluginClassLoaders.remove(description.getClassLoader()); description.clearClassLoader();