Concurrently Saving FileConfiguration Fails

Hello!

I've recently run across an annoying speed bump which refuses to get out of the way, no matter that I try. I want per-world configuration files for my plugin. Not too hard, in theory.

I have an object which loads and saves the file. Its really like:

Code:Java

 
GlobalVariables config = new GlobalVariables(this, "test");
config.load();
 

And that's it! That will create a file called test.yml, and populate it with various things. What I am doing is this:

Code:Java

 
for (World w : Bukkit.getWorlds()) {
String world = w.getName();
GlobalVariables config = new GlobalVariables(this, world);
config.load();
wConfigs.put(world, config);
}
 

Expected result: my plugin folder being filled with config files of each world's name. What actually happens: only world.yml (or whatever the first world it finds is) is generated.

My hypothesis is that it cannot save files at the same time: that the second world tries to be saved before the first finishes (possible lack of BufferedOutputStream *).

And so I tried this:

Code:Java

 
Bukkit.getScheduler()
.scheduleAsyncDelayedTask(
plugin,
new Runnable() {
@Override
public void run() {
try {
finalWorlds.save(finalWorldFile);
} catch (IOException e) {
e.printStackTrace();
}
}
}, 20);
 

Still doesn't work. Increasing the delay to around 500 ticks works, but its a messy solution: I hope there is a better way.

I also tried using a mutex:

Code:Java

 
Semaphore mutex = new Semaphore(1);
.....
mutex.acquire();
finalWorlds.save(finalWorldFile);
mutex.release();
 

Alas, not even that worked!

Anyone have any ideas, or am I stuck using a 500 tick delay?

* I doubt this, however. I looked at the save() method, and it just calls the JRE FileWriter class. I'm reasonably sure they would make that use a buffer.

 

You could try and use this class:

PHP:

package net.milkycraft;
 
import java.io.File;
import java.io.IOException;
 
import org.bukkit.configuration.file.FileConfiguration;
import org.bukkit.configuration.file.YamlConfiguration;
 
/**
* The Class ConfigLoader.
*/
public abstract class ConfigLoader {
 
    /** The file name. */
    protected String fileName;
 
    /** The config file. */
    protected File configFile;
 
    /** The data folder. */
    protected File dataFolder;
 
    /** The plugin. */
    protected final MainClass plugin;
 
    /** The config. */
    protected static FileConfiguration config;
 
    /**
    * Instantiates a new config loader.
    *
    * @param plugin
    *            the plugin
    * @param fileName
    *            the file name
    */
    public ConfigLoader(MainClass plugin, String fileName) {
        this.plugin = plugin;
        this.fileName = fileName;
        this.dataFolder = plugin.getDataFolder();
        this.configFile = new File(this.dataFolder, File.separator + fileName);
        config = YamlConfiguration.loadConfiguration(this.configFile);
    }
 
    /**
    * Load this config file.
    */
    public void load() {
        if (!this.configFile.exists()) {
            this.dataFolder.mkdir();
            saveConfig();
        }
        addDefaults();
        loadKeys();
    }
 
    /**
    * Save this config file.
    */
    protected void saveConfig() {
        try {
            config.save(this.configFile);
        } catch (IOException ex) {
        }
    }
 
    /**
    * Save if not exist.
    */
    protected void saveIfNotExist() {
        if (!this.configFile.exists()) {
            if (this.plugin.getResource(this.fileName) != null) {
                this.plugin.saveResource(this.fileName, false);
            }
        }
        rereadFromDisk();
    }
 
    /**
    * Reread from disk.
    */
    protected void rereadFromDisk() {
        config = YamlConfiguration.loadConfiguration(this.configFile);
    }
 
    /**
    * Add the defaults to this config file.
    */
    protected void addDefaults() {
        config.options().copyDefaults(true);
        saveConfig();
    }
 
    /**
    * Load the keys from this config file.
    */
    protected abstract void loadKeys();
 
}

In conjunction with this class

Code:

package net.milkycraft;
 
import org.bukkit.configuration.file.FileConfiguration;
/**
* The Class Settings.
*/
public class Settings extends ConfigLoader {
    public static Settings instance;
    public static String command;
    public static String password;
 
    /**
    * Instantiates a new settings.
    *
    * @param plugin
    *            the plugin
    */
    public Settings(MainClass plugin) {
        super(plugin, "config.yml");
        instance = this;
        config = plugin.getConfig();
        super.saveIfNotExist();
    }
 
    /*
    * (non-Javadoc)
    *
    * @see net.milkycraft.configuration.ConfigLoader#load()
    */
    @Override
    public void load() {
        // If it doesn't exist, copy it from the .jar
        if (!configFile.exists()) {
            dataFolder.mkdir();
            plugin.saveDefaultConfig();
        }
        super.addDefaults();
        loadKeys();
    }
 
    /*
    * (non-Javadoc)
    *
    * @see net.milkycraft.configuration.ConfigLoader#loadKeys()
    */
    @Override
    public void loadKeys() {
        command = config.getString("Settings.Command");
        password = config.getString("Settings.Password");
    }
 
    /**
    * Gets the config.
    *
    * @return Return the config
    */
    public static FileConfiguration getConfig() {
        return config;
    }
 
    public static Settings getSettings() {
        return instance;
    }
}

I can see you using the code in your application by using the settings constructor to create a new config file for every world, I'm sure if you played around with it a little, it would work nicely.

 

Milkywayz Thanks for your reply! It would be strange if your method worked when mine didn't as the are essentially the same, but I'll try it anyways. My method:

Code:Java

 
package com.github.StormTeam.Storm;
 
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.concurrent.Semaphore;
 
import org.bukkit.Bukkit;
import org.bukkit.configuration.file.FileConfiguration;
import org.bukkit.configuration.file.YamlConfiguration;
import org.bukkit.plugin.Plugin;
 
public class ReflectConfiguration {
 
    /*
     * Based on codename_B's non static config 'offering' :-)
     */
    private Plugin plugin;
    private String name;
    private Semaphore mutex = new Semaphore(1);
 
    public ReflectConfiguration(Plugin storm, String name) {
        this.plugin = storm;
        this.name = name;
    }
 
    public void load() {
 
        try {
            onLoad(plugin);
        } catch (Exception e) {
            e.printStackTrace();
        }
 
    }
 
    private void onLoad(Plugin plugin) throws Exception {
 
        File worldFile = new File(plugin.getDataFolder(), name + ".yml");
 
        FileConfiguration worlds = YamlConfiguration
                .loadConfiguration(worldFile);
 
        for (Field field : getClass().getDeclaredFields()) {
            String path = "Storm."
                    + field.getName().replaceAll("__", " ")
                    .replaceAll("_", ".");
            if (doSkip(field)) {
            } else if (worlds.isSet(path)) {
                field.set(this, worlds.get(path));
            } else {
                worlds.set(path, field.get(this));
            }
        }
 
        mutex.acquire();
        worlds.save(worldFile);        
        mutex.release();
    }
 
    private boolean doSkip(Field field) {
        int mod = field.getModifiers();
        return Modifier.isTransient(mod)
                || Modifier.isStatic(mod)
                || Modifier.isFinal(mod)
                || Modifier.isPrivate(mod);
    }
}
 
 

GlobalVariables just extends it.

 

Yeah my method wouldn't be exact, it'd be something to get the brain thinking on a solution. If you figure it out, post it here so it can help others. Also I would suggest checking out how worldguard does it, since it does it quite well

 

My hypothesis: my plugin appears to load before MultiVerse... If MV hasn't loaded the worlds its responsible for, world WOULD be the only world loaded (world_nether and _the_end are disabled)... Is there a way to load my plugin AFTER MV?

 

You could try adding the following to your plugin.yml

Code:

softdepend: [MultiVerse]

plugin.yml options

 

Also you could listen to the WorldLoadEvent for worlds that are loaded after your plugin. That would be compatible with all and every world loading plugin.

 

You developed a hypothesis! Great, now this is where you conduct your experiment based upon your hypothesis:

Code:

where problem is happening {
    System.out.println(Bukkit.getServer().getWorlds());
}

Great, you tested it! Now publish your results.

 

Too busy with homework at the moment Will test when I get the time

 

chaseoes Have 38 pages of writing to do for English today, but I managed to get time to test it. It works! The config files are properly generated, with no deformations. It led to some complexity addition, as all my config objects are stored in a HashMap<World, ConfigObject>. Now I have to see if it .containsKey before .get ing, but aside from that it works like a charm.

Thanks to everyone who helped!

 

Shouldn't you use a HashMap<String, ConfigObject>? Otherwise you might get the same situation as with saving a Player to a Collection.

 

No, you do not; world objects are never fully unloaded anyways, unlike players. When a player logs out, its instance is useless. If you reference it, then GC can't clean it up. A world object is never unloaded, even when there are no players inside the world; it goes idle. So there is a big difference between changing a evanescent player object vs a never-unloaded world object.

If it was HashMap<Player, ConfigObject>, now that would be a problem.

 

Umm, you know that world can in fact be ... unloaded?
Actually, if you prevent a CraftWorld from being collected when its world has been unloaded, that's a far worse issue than doing the same with a player (and the player left). The player entity itself isn't that bad if it stays in memory, stuff gets nasty when we are talking about chunk data - which happens to be inside worlds.

So if you unload a world but keep a reference to it, it can never be collected. If you re-load it, the old chunks will still be in memory, in addition to the new ones.

Storing "World" is BAD. If you have to do it, at least listen to WorldUnloadEvent, and/or use a WeakReference.
Preferably use its name or UUID, though.

 

Bone008 Under what circumstances is a world unloaded?

 

When a plugin calls Bukkit(.getServer()).unloadWorld(...).
A generic world managing plugin like MultiVerse usually does that by command ("/mv unload <world>" maybe). Imagine someone could unload the world, change some files in it around and load it back up again.

More specific plugins providing special kinds of worlds might do that automatically.
For example, I am developping a plugin that allows creating virtual (read-only) worlds that only exist in memory and are disposed when being left (mainly for minigames, adventure maps, sandboxes, etc.).
If you populate your Map with each of these worlds but you don't clear them, all the chunk data will add up in memory, causing one of the biggest memory leaks that can happen in minecraft (I don't think there's anything else that takes up as much RAM as chunks).

 

Bone008 One learns something new every day. Thanks! I've made a small listener which just removes world keys from the map. Not as clean as using a UUID, but I prefer it over having to do HashMap.containsKey(world.getUUID()) or whatever the method to get the UUID is.