summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsnowleo <schneeleo@gmail.com>2011-12-06 14:39:52 +0100
committersnowleo <schneeleo@gmail.com>2011-12-06 14:39:52 +0100
commit51390a969809f779bd867d697e3b7950fdf2740c (patch)
tree023ac08ce3eb88e6a3c457fd7191305ccfdb1d4e
parent019b49ef11b453026e63b9e683d7827b85c81ab9 (diff)
downloadEssentials-51390a969809f779bd867d697e3b7950fdf2740c.tar
Essentials-51390a969809f779bd867d697e3b7950fdf2740c.tar.gz
Essentials-51390a969809f779bd867d697e3b7950fdf2740c.tar.lz
Essentials-51390a969809f779bd867d697e3b7950fdf2740c.tar.xz
Essentials-51390a969809f779bd867d697e3b7950fdf2740c.zip
Prevent some rare cases of NPE and Deadlocks, better error handling on yaml load
-rw-r--r--Essentials/src/com/earth2me/essentials/storage/AbstractDelayedYamlFileReader.java19
-rw-r--r--Essentials/src/com/earth2me/essentials/storage/IStorageReader.java2
-rw-r--r--Essentials/src/com/earth2me/essentials/storage/ObjectLoadException.java10
-rw-r--r--Essentials/src/com/earth2me/essentials/storage/YamlStorageReader.java43
-rw-r--r--Essentials/src/com/earth2me/essentials/user/User.java1
-rw-r--r--Essentials/test/com/earth2me/essentials/StorageTest.java150
-rw-r--r--EssentialsSpawn/src/com/earth2me/essentials/spawn/EssentialsSpawn.java1
-rw-r--r--EssentialsSpawn/src/com/earth2me/essentials/spawn/SpawnStorage.java22
8 files changed, 144 insertions, 104 deletions
diff --git a/Essentials/src/com/earth2me/essentials/storage/AbstractDelayedYamlFileReader.java b/Essentials/src/com/earth2me/essentials/storage/AbstractDelayedYamlFileReader.java
index 0bac39bdb..1f2887d88 100644
--- a/Essentials/src/com/earth2me/essentials/storage/AbstractDelayedYamlFileReader.java
+++ b/Essentials/src/com/earth2me/essentials/storage/AbstractDelayedYamlFileReader.java
@@ -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();
}
diff --git a/Essentials/src/com/earth2me/essentials/storage/IStorageReader.java b/Essentials/src/com/earth2me/essentials/storage/IStorageReader.java
index 5e259a322..d59adafe0 100644
--- a/Essentials/src/com/earth2me/essentials/storage/IStorageReader.java
+++ b/Essentials/src/com/earth2me/essentials/storage/IStorageReader.java
@@ -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;
}
diff --git a/Essentials/src/com/earth2me/essentials/storage/ObjectLoadException.java b/Essentials/src/com/earth2me/essentials/storage/ObjectLoadException.java
new file mode 100644
index 000000000..8b804abc3
--- /dev/null
+++ b/Essentials/src/com/earth2me/essentials/storage/ObjectLoadException.java
@@ -0,0 +1,10 @@
+package com.earth2me.essentials.storage;
+
+
+public class ObjectLoadException extends Exception
+{
+ public ObjectLoadException(Throwable thrwbl)
+ {
+ super(thrwbl);
+ }
+}
diff --git a/Essentials/src/com/earth2me/essentials/storage/YamlStorageReader.java b/Essentials/src/com/earth2me/essentials/storage/YamlStorageReader.java
index 3aa483e5c..5d1ff668a 100644
--- a/Essentials/src/com/earth2me/essentials/storage/YamlStorageReader.java
+++ b/Essentials/src/com/earth2me/essentials/storage/YamlStorageReader.java
@@ -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;
}
- finally
+ catch (Exception ex)
{
- lock.unlock();
+ throw new ObjectLoadException(ex);
}
- if (ret == null)
+ finally
{
- 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);
- }
+ lock.unlock();
}
- return ret;
}
private Constructor prepareConstructor(final Class<?> clazz)
diff --git a/Essentials/src/com/earth2me/essentials/user/User.java b/Essentials/src/com/earth2me/essentials/user/User.java
index 70f2c7f3c..55740134f 100644
--- a/Essentials/src/com/earth2me/essentials/user/User.java
+++ b/Essentials/src/com/earth2me/essentials/user/User.java
@@ -81,7 +81,6 @@ public class User extends UserBase implements IOfflineUser
new UserDataWriter();
}
-
private class UserDataWriter extends AbstractDelayedYamlFileWriter
{
public UserDataWriter()
diff --git a/Essentials/test/com/earth2me/essentials/StorageTest.java b/Essentials/test/com/earth2me/essentials/StorageTest.java
index d4dc1be70..b7fe23433 100644
--- a/Essentials/test/com/earth2me/essentials/StorageTest.java
+++ b/Essentials/test/com/earth2me/essentials/StorageTest.java
@@ -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)");
+ try
+ {
+ 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++)
+ 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)
{
- userdata.getHomes().put("home", new Location(world, j, j, j));
+ 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();
}
diff --git a/EssentialsSpawn/src/com/earth2me/essentials/spawn/EssentialsSpawn.java b/EssentialsSpawn/src/com/earth2me/essentials/spawn/EssentialsSpawn.java
index 6f6244f25..ad42f9dee 100644
--- a/EssentialsSpawn/src/com/earth2me/essentials/spawn/EssentialsSpawn.java
+++ b/EssentialsSpawn/src/com/earth2me/essentials/spawn/EssentialsSpawn.java
@@ -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);
diff --git a/EssentialsSpawn/src/com/earth2me/essentials/spawn/SpawnStorage.java b/EssentialsSpawn/src/com/earth2me/essentials/spawn/SpawnStorage.java
index ac98b5921..02fb26c9b 100644
--- a/EssentialsSpawn/src/com/earth2me/essentials/spawn/SpawnStorage.java
+++ b/EssentialsSpawn/src/com/earth2me/essentials/spawn/SpawnStorage.java
@@ -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)
+ {
+ if (object != null)
+ {
+ spawns = object;
+ }
+ rwl.writeLock().unlock();
+ }
+
+ @Override
+ public void onException()
{
- spawns = object;
+ if (spawns == null) {
+ spawns = new Spawns();
+ }
rwl.writeLock().unlock();
}
}