Prevent some rare cases of NPE and Deadlocks, better error handling on yaml load

This commit is contained in:
snowleo 2011-12-06 14:39:52 +01:00
parent 019b49ef11
commit 51390a9698
8 changed files with 145 additions and 105 deletions

View file

@ -8,6 +8,7 @@ import java.io.IOException;
import java.util.logging.Level;
import org.bukkit.Bukkit;
import org.bukkit.plugin.Plugin;
import org.yaml.snakeyaml.error.YAMLException;
public abstract class AbstractDelayedYamlFileReader<T extends StorageObject> implements Runnable
@ -33,9 +34,17 @@ public abstract class AbstractDelayedYamlFileReader<T extends StorageObject> imp
try
{
onStart();
reader = new FileReader(file);
final T object = new YamlStorageReader(reader, plugin).load(clazz);
onFinish(object);
try
{
reader = new FileReader(file);
T object = new YamlStorageReader(reader, plugin).load(clazz);
onSuccess(object);
}
catch (ObjectLoadException ex)
{
onException();
Bukkit.getLogger().log(Level.SEVERE, "File broken: " + file.toString(), ex.getCause());
}
}
catch (FileNotFoundException ex)
{
@ -57,5 +66,7 @@ public abstract class AbstractDelayedYamlFileReader<T extends StorageObject> imp
}
}
public abstract void onFinish(T object);
public abstract void onSuccess(T object);
public abstract void onException();
}

View file

@ -3,5 +3,5 @@ package com.earth2me.essentials.storage;
public interface IStorageReader
{
<T extends StorageObject> T load(final Class<? extends T> clazz);
<T extends StorageObject> T load(final Class<? extends T> clazz) throws ObjectLoadException;
}

View file

@ -0,0 +1,10 @@
package com.earth2me.essentials.storage;
public class ObjectLoadException extends Exception
{
public ObjectLoadException(Throwable thrwbl)
{
super(thrwbl);
}
}

View file

@ -4,8 +4,6 @@ import java.io.Reader;
import java.lang.reflect.Field;
import java.util.*;
import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.bukkit.plugin.Plugin;
import org.yaml.snakeyaml.TypeDescription;
import org.yaml.snakeyaml.Yaml;
@ -14,8 +12,8 @@ import org.yaml.snakeyaml.constructor.Constructor;
public class YamlStorageReader implements IStorageReader
{
private transient static Map<Class, Yaml> preparedYamls = Collections.synchronizedMap(new HashMap<Class, Yaml>());
private transient static Map<Class, ReentrantLock> locks = new HashMap<Class, ReentrantLock>();
private transient static final Map<Class, Yaml> PREPARED_YAMLS = Collections.synchronizedMap(new HashMap<Class, Yaml>());
private transient static final Map<Class, ReentrantLock> LOCKS = new HashMap<Class, ReentrantLock>();
private transient final Reader reader;
private transient final Plugin plugin;
@ -26,49 +24,40 @@ public class YamlStorageReader implements IStorageReader
}
@Override
public <T extends StorageObject> T load(final Class<? extends T> clazz)
public <T extends StorageObject> T load(final Class<? extends T> clazz) throws ObjectLoadException
{
Yaml yaml = preparedYamls.get(clazz);
Yaml yaml = PREPARED_YAMLS.get(clazz);
if (yaml == null)
{
yaml = new Yaml(prepareConstructor(clazz));
preparedYamls.put(clazz, yaml);
PREPARED_YAMLS.put(clazz, yaml);
}
ReentrantLock lock;
synchronized (locks)
synchronized (LOCKS)
{
lock = locks.get(clazz);
lock = LOCKS.get(clazz);
if (lock == null)
{
lock = new ReentrantLock();
}
}
T ret;
lock.lock();
try
{
ret = (T)yaml.load(reader);
T object = (T)yaml.load(reader);
if (object == null) {
object = clazz.newInstance();
}
return object;
}
catch (Exception ex)
{
throw new ObjectLoadException(ex);
}
finally
{
lock.unlock();
}
if (ret == null)
{
try
{
ret = (T)clazz.newInstance();
}
catch (InstantiationException ex)
{
Logger.getLogger(StorageObject.class.getName()).log(Level.SEVERE, null, ex);
}
catch (IllegalAccessException ex)
{
Logger.getLogger(StorageObject.class.getName()).log(Level.SEVERE, null, ex);
}
}
return ret;
}
private Constructor prepareConstructor(final Class<?> clazz)

View file

@ -81,7 +81,6 @@ public class User extends UserBase implements IOfflineUser
new UserDataWriter();
}
private class UserDataWriter extends AbstractDelayedYamlFileWriter
{
public UserDataWriter()

View file

@ -1,6 +1,7 @@
package com.earth2me.essentials;
import com.earth2me.essentials.settings.Settings;
import com.earth2me.essentials.storage.ObjectLoadException;
import com.earth2me.essentials.storage.StorageObject;
import com.earth2me.essentials.storage.YamlStorageReader;
import com.earth2me.essentials.storage.YamlStorageWriter;
@ -11,7 +12,6 @@ import org.bukkit.World;
import org.bukkit.World.Environment;
import org.bukkit.plugin.InvalidDescriptionException;
import org.junit.Test;
import org.yaml.snakeyaml.Yaml;
public class StorageTest extends TestCase
@ -42,82 +42,94 @@ public class StorageTest extends TestCase
@Test
public void testSettings()
{
assertTrue(StorageObject.class.isAssignableFrom(Settings.class));
ExecuteTimer ext = new ExecuteTimer();
ext.start();
final ByteArrayInputStream bais = new ByteArrayInputStream(new byte[0]);
final Reader reader = new InputStreamReader(bais);
final Settings settings = new YamlStorageReader(reader, null).load(Settings.class);
ext.mark("load empty settings");
final ByteArrayInputStream bais3 = new ByteArrayInputStream(new byte[0]);
final Reader reader3 = new InputStreamReader(bais3);
final Settings settings3 = new YamlStorageReader(reader3, null).load(Settings.class);
ext.mark("load empty settings (class cached)");
final ByteArrayOutputStream baos = new ByteArrayOutputStream();
final PrintWriter writer = new PrintWriter(baos);
new YamlStorageWriter(writer).save(settings);
writer.close();
ext.mark("write settings");
byte[] written = baos.toByteArray();
System.out.println(new String(written));
final ByteArrayInputStream bais2 = new ByteArrayInputStream(written);
final Reader reader2 = new InputStreamReader(bais2);
final Settings settings2 = new YamlStorageReader(reader2, null).load(Settings.class);
System.out.println(settings.toString());
System.out.println(settings2.toString());
ext.mark("reload settings");
System.out.println(ext.end());
//assertEquals("Default and rewritten config should be equal", settings, settings2);
//that assertion fails, because empty list and maps return as null
try
{
assertTrue(StorageObject.class.isAssignableFrom(Settings.class));
ExecuteTimer ext = new ExecuteTimer();
ext.start();
final ByteArrayInputStream bais = new ByteArrayInputStream(new byte[0]);
final Reader reader = new InputStreamReader(bais);
final Settings settings = new YamlStorageReader(reader, null).load(Settings.class);
ext.mark("load empty settings");
final ByteArrayInputStream bais3 = new ByteArrayInputStream(new byte[0]);
final Reader reader3 = new InputStreamReader(bais3);
final Settings settings3 = new YamlStorageReader(reader3, null).load(Settings.class);
ext.mark("load empty settings (class cached)");
final ByteArrayOutputStream baos = new ByteArrayOutputStream();
final PrintWriter writer = new PrintWriter(baos);
new YamlStorageWriter(writer).save(settings);
writer.close();
ext.mark("write settings");
byte[] written = baos.toByteArray();
System.out.println(new String(written));
final ByteArrayInputStream bais2 = new ByteArrayInputStream(written);
final Reader reader2 = new InputStreamReader(bais2);
final Settings settings2 = new YamlStorageReader(reader2, null).load(Settings.class);
System.out.println(settings.toString());
System.out.println(settings2.toString());
ext.mark("reload settings");
System.out.println(ext.end());
//assertEquals("Default and rewritten config should be equal", settings, settings2);
//that assertion fails, because empty list and maps return as null
}
catch (ObjectLoadException ex)
{
fail(ex.getMessage());
}
}
@Test
public void testUserdata()
{
FakeServer server = new FakeServer();
World world = server.createWorld("testWorld", Environment.NORMAL);
ExecuteTimer ext = new ExecuteTimer();
ext.start();
final ByteArrayInputStream bais = new ByteArrayInputStream(new byte[0]);
final Reader reader = new InputStreamReader(bais);
final com.earth2me.essentials.user.UserData userdata = new YamlStorageReader(reader, null).load(com.earth2me.essentials.user.UserData.class);
ext.mark("load empty user");
final ByteArrayInputStream bais3 = new ByteArrayInputStream(new byte[0]);
final Reader reader3 = new InputStreamReader(bais3);
final com.earth2me.essentials.user.UserData userdata3 = new YamlStorageReader(reader3, null).load(com.earth2me.essentials.user.UserData.class);
ext.mark("load empty user (class cached)");
for (int j = 0; j < 10000; j++)
try
{
userdata.getHomes().put("home", new Location(world, j, j, j));
FakeServer server = new FakeServer();
World world = server.createWorld("testWorld", Environment.NORMAL);
ExecuteTimer ext = new ExecuteTimer();
ext.start();
final ByteArrayInputStream bais = new ByteArrayInputStream(new byte[0]);
final Reader reader = new InputStreamReader(bais);
final com.earth2me.essentials.user.UserData userdata = new YamlStorageReader(reader, null).load(com.earth2me.essentials.user.UserData.class);
ext.mark("load empty user");
final ByteArrayInputStream bais3 = new ByteArrayInputStream(new byte[0]);
final Reader reader3 = new InputStreamReader(bais3);
final com.earth2me.essentials.user.UserData userdata3 = new YamlStorageReader(reader3, null).load(com.earth2me.essentials.user.UserData.class);
ext.mark("load empty user (class cached)");
for (int j = 0; j < 10000; j++)
{
userdata.getHomes().put("home", new Location(world, j, j, j));
}
ext.mark("change home 10000 times");
final ByteArrayOutputStream baos = new ByteArrayOutputStream();
final PrintWriter writer = new PrintWriter(baos);
new YamlStorageWriter(writer).save(userdata);
writer.close();
ext.mark("write user");
final ByteArrayOutputStream baos2 = new ByteArrayOutputStream();
final PrintWriter writer2 = new PrintWriter(baos2);
new YamlStorageWriter(writer2).save(userdata);
writer2.close();
ext.mark("write user (cached)");
byte[] written = baos.toByteArray();
System.out.println(new String(written));
ext.mark("debug output");
final ByteArrayInputStream bais2 = new ByteArrayInputStream(written);
final Reader reader2 = new InputStreamReader(bais2);
final com.earth2me.essentials.user.UserData userdata2 = new YamlStorageReader(reader2, null).load(com.earth2me.essentials.user.UserData.class);
ext.mark("reload file");
final ByteArrayInputStream bais4 = new ByteArrayInputStream(written);
final Reader reader4 = new InputStreamReader(bais4);
final com.earth2me.essentials.user.UserData userdata4 = new YamlStorageReader(reader4, null).load(com.earth2me.essentials.user.UserData.class);
ext.mark("reload file (cached)");
System.out.println(userdata.toString());
System.out.println(userdata2.toString());
System.out.println(ext.end());
}
catch (ObjectLoadException ex)
{
fail(ex.getMessage());
}
ext.mark("change home 10000 times");
final ByteArrayOutputStream baos = new ByteArrayOutputStream();
final PrintWriter writer = new PrintWriter(baos);
new YamlStorageWriter(writer).save(userdata);
writer.close();
ext.mark("write user");
final ByteArrayOutputStream baos2 = new ByteArrayOutputStream();
final PrintWriter writer2 = new PrintWriter(baos2);
new YamlStorageWriter(writer2).save(userdata);
writer2.close();
ext.mark("write user (cached)");
byte[] written = baos.toByteArray();
System.out.println(new String(written));
ext.mark("debug output");
final ByteArrayInputStream bais2 = new ByteArrayInputStream(written);
final Reader reader2 = new InputStreamReader(bais2);
final com.earth2me.essentials.user.UserData userdata2 = new YamlStorageReader(reader2, null).load(com.earth2me.essentials.user.UserData.class);
ext.mark("reload file");
final ByteArrayInputStream bais4 = new ByteArrayInputStream(written);
final Reader reader4 = new InputStreamReader(bais4);
final com.earth2me.essentials.user.UserData userdata4 = new YamlStorageReader(reader4, null).load(com.earth2me.essentials.user.UserData.class);
ext.mark("reload file (cached)");
System.out.println(userdata.toString());
System.out.println(userdata2.toString());
System.out.println(ext.end());
com.earth2me.essentials.user.User test = new com.earth2me.essentials.user.User(null, ess);
test.example();
}

View file

@ -35,6 +35,7 @@ public class EssentialsSpawn extends JavaPlugin
}
spawns = new SpawnStorage(ess);
ess.addReloadListener(spawns);
final EssentialsSpawnPlayerListener playerListener = new EssentialsSpawnPlayerListener(ess, spawns);
pluginManager.registerEvent(Type.PLAYER_RESPAWN, playerListener, Priority.Low, this);

View file

@ -6,12 +6,14 @@ import com.earth2me.essentials.IEssentialsModule;
import com.earth2me.essentials.settings.Spawns;
import com.earth2me.essentials.storage.AbstractDelayedYamlFileReader;
import com.earth2me.essentials.storage.AbstractDelayedYamlFileWriter;
import com.earth2me.essentials.storage.ObjectLoadException;
import com.earth2me.essentials.storage.StorageObject;
import java.io.File;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.bukkit.Bukkit;
import org.bukkit.Location;
import org.bukkit.World;
@ -35,6 +37,10 @@ public class SpawnStorage implements IConf, IEssentialsModule
rwl.writeLock().lock();
try
{
if (spawns == null)
{
spawns = new Spawns();
}
if (spawns.getSpawns() == null)
{
spawns.setSpawns(new HashMap<String, Location>());
@ -136,9 +142,21 @@ public class SpawnStorage implements IConf, IEssentialsModule
}
@Override
public void onFinish(final Spawns object)
public void onSuccess(final Spawns object)
{
spawns = object;
if (object != null)
{
spawns = object;
}
rwl.writeLock().unlock();
}
@Override
public void onException()
{
if (spawns == null) {
spawns = new Spawns();
}
rwl.writeLock().unlock();
}
}