Plugin Disable Order Changes

Discussion in 'Plugin Development' started by Wolvereness, May 25, 2012.

Thread Status:
Not open for further replies.
  1. Offline

    Wolvereness Bukkit Team Member

    In the current 1.2.5-R4 Recommended Build, plugins are disabled in reverse order from when they were enabled (builds #2205 onward).

    So, I'd like some feedback on your experiences with the previous disable order in comparison to the new disable order. The changes were designed to address issues with class loaders (which were made predictable in the latest recommended build, 1.2.5-R3.0) and dependencies not being available during disable routines.
     
  2. Offline

    md_5

    Makes sense to me. Are you thinking this will cause issues?
     
  3. Offline

    megabytes

    I had the same thoughts, md_5. Seems like a perfectly reasonable and necessary change and will definitely help in some cases in my own plugin dependencies. I don't see why this needs to be discussed? :)
     
  4. Offline

    PandazNWafflez

    All plugins I have made for private use (haven't released any with dependencies) listen for the plugins they depend on disabling and disable themselves. Would this break the order as it would hence disable too early? (obviously the other plugins load first because of depend: http://dev.bukkit.org/bukkit-plugins/Not Provided/
     
  5. Offline

    Wolvereness Bukkit Team Member

     
  6. Offline

    ST-DDT

    I'm not sure whether reversing the disabling order solves the problem of missing classes.
    I'll test it. But I guess if you have two plugins Plugin and Plugin_Addin. Plugin_Addin depends on Plugin.
    Then the code could contain a list of "Saveable" Objects. Some of them may be added in Plugin_Addin (being a class defined in Plugin_Addin), but are saved in Plugin. This may cause issues.

    I thought about another solution. (The ticket is mine)
    You could create a HashMap entry on pluign loading

    Code:
    protected static Map<String,String> depencies=new Map<String,String>();
     
    pluginEnable
    {
    [...]
    foreach (depency)
        depencies.get(depency.getName()).add(this.getName());
    depencies.put(this.getName(),new List<String>());
    [...]
    }
     
    pluginDisable
    {
    [...]
    foreach (depency)
        {
        depencies.get(depency.getName()).remove(this.getName());
        if ( depencies.get(depency.getName()).size()==0)
            {
            depency.unloadClasses();
            depencies.remove(depency.getName());
            }
        }
    if (depencies.get(this.getName()).size()==0)
        {
        unloadClasses();
        depencies.remove(this.getName());
        }
    }
    
    Shall i create a pullrequest with proper code?
     
  7. Offline

    Wolvereness Bukkit Team Member

    Effectively, that's what's happening now, because dependencies are causing a plugin to load with dependent after depended, it will unload dependent before depended. The issue with depended saving classes from something getting disabled is not an issue. The class loader design will always do a self-reference to finally find a class (in this case, the calling classloader is really the one of the orphaned plugin).

    TL;DR, there should be no further issues unless there is an incorrect dependency hierarchy.
     
  8. Offline

    ST-DDT

    Maybe I got you wrong, but this example should fail. Cannot test it properly because class unloading happens in another thread I guess.

    Plugin1:
    P1Main+Saveable
    Plugin2 depends on Plugin1:
    P2Main+Storage implements Saveable+SaveHelper

    P1Main:
    static getP1Main()
    addSaveable(saveable);
    onDisable()
    {
    for (Saveable save:savelist)
    save.save(config);
    }

    P2Main:
    onEnable()
    {
    getP1Main().addSaveable(new Storage());
    }

    Storage:
    save(config)
    {
    help=new SaveHelper();
    help.foo();
    config.set("help",help.toString());
    config.set(this.getName(),this.toString());
    }

    Old disable order:
    Should cause no errors
    New disable order:
    Storage + SaveHelper already disabled (ERROR)
    My version (with old order) no errors

    Or does this plugin something wrong?
     
  9. Offline

    Wolvereness Bukkit Team Member

    With the new version, Plugin2.Storage is referenced from Plugin1. Plugin1 executes method on Plugin2, but Plugin2 is disabled. Anything referenced from Plugin2.Storage will still have a fallback to the Plugin2.class.getClassLoader(). Plugin1 cannot 'directly' access anything in Plugin2 at this state, but Plugin2 objects have their own 'scope' as defined by the class loader, and the scope will still fall back to its own classloader (now dereferenced from JavaPluginLoader).

    If you cannot 'save' after you have been disabled, as far as your implementation, then that should be corrected with the new disable order in mind.
     
  10. Offline

    ferrybig

    so the new order is:
    1: enable
    2: enable
    3: enable
    3: disable
    2: disable
    1: disable
    ?
     
  11. Offline

    Wolvereness Bukkit Team Member

    Yes
     
  12. Offline

    ZachBora

    I always wondered why it wasn't like this in the first place.
     
Thread Status:
Not open for further replies.

Share This Page