Allow plugins to inhibit (un)loading.

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);

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
      */

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;
+    }
+}

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;
 }

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);
+            }
         }
     }
 

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;
     }
 
     /**

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<? extends JavaPlugin> constructor = pluginClass.getConstructor(Server.class, JavaPluginDescription.class);
-                plugin = constructor.newInstance(server, description);
-            } catch (NoSuchMethodException ex) {
-                Constructor<? extends JavaPlugin> constructor = pluginClass.getConstructor();
-                plugin = constructor.newInstance();
+                try {
+                    Constructor<? extends JavaPlugin> constructor = pluginClass.getConstructor(Server.class, JavaPluginDescription.class);
+                    plugin = constructor.newInstance(server, description);
+                } catch (NoSuchMethodException ex) {
+                    Constructor<? extends JavaPlugin> 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<Command> 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<Command> 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();