Log if mods do non-threadsafe manual chunk unloading or loading.

Signed-off-by: Ross Allan <rallanpcl@gmail.com>
MinimallyCorrect · Jun 20, 2013 · bddd423 · bddd423
1 parent 4871c1d
commit bddd423
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 45 deletions.
diff --git a/src/common/nallar/patched/forge/PatchDimensionManager.java b/src/common/nallar/patched/forge/PatchDimensionManager.java
@@ -53,14 +53,15 @@ public static synchronized boolean unloadWorld(WorldServer w, boolean save) {
 			return false;
 		}
 
-		if (save) {
-			try {
-				w.saveAllChunks(true, null);
-				w.flush();
-				fireBukkitWorldSave(w);
-			} catch (net.minecraft.world.MinecraftException ex) {
-				FMLLog.log(Level.SEVERE, ex, "Failed to save world " + w.getName() + " while unloading it.");
-			}
+		if (!save) {
+			Log.severe("Requested to unload a world without saving it. Ignoring this.");
+		}
+		try {
+			w.saveAllChunks(true, null);
+			w.flush();
+			fireBukkitWorldSave(w);
+		} catch (net.minecraft.world.MinecraftException ex) {
+			FMLLog.log(Level.SEVERE, ex, "Failed to save world " + w.getName() + " while unloading it.");
 		}
 		removeBukkitWorld(w);
 		try {

diff --git a/src/common/nallar/patched/forge/PatchPacketDispatcher.java b/src/common/nallar/patched/forge/PatchPacketDispatcher.java
@@ -1,6 +1,5 @@
 package nallar.patched.forge;
 
-import cpw.mods.fml.common.network.Player;
 import nallar.tickthreading.patcher.Declare;
 import net.minecraft.entity.player.EntityPlayerMP;
 import net.minecraft.network.packet.Packet;

diff --git a/src/common/nallar/patched/server/PatchMinecraftServer.java b/src/common/nallar/patched/server/PatchMinecraftServer.java
@@ -47,7 +47,6 @@
 import net.minecraftforge.common.DimensionManager;
 import net.minecraftforge.common.MinecraftForge;
 import net.minecraftforge.event.world.WorldEvent;
-import sun.misc.Unsafe;
 
 public abstract class PatchMinecraftServer extends MinecraftServer {
 	private ThreadManager threadManager;
@@ -242,12 +241,16 @@ public void run() {
 					Thread.sleep(100L);
 				} catch (InterruptedException ignored) {
 				}
-				this.saveEverything();
+				try {
+					saveEverything(true);
+				} catch (Throwable t) {
+					Log.severe("Failed to perform save after crash", t);
+				}
 				this.finalTick(crashReport);
 			}
 		} finally {
 			try {
-				saveEverything();
+				saveEverything(true);
 			} catch (Throwable t) {
 				Log.severe("Failed to perform shutdown save.", t);
 			}
@@ -604,7 +607,13 @@ public void doNetworkTicks() {
 	@Override
 	@Declare
 	public void saveEverything() {
-		if (this.isServerRunning() && currentlySaving.get() == 0) {
+		saveEverything(false);
+	}
+
+	@Override
+	@Declare
+	public void saveEverything(boolean force) {
+		if (force || (this.isServerRunning() && currentlySaving.get() == 0)) {
 			currentlySaving.getAndIncrement();
 			try {
 				Log.info("Saving all player data.");

diff --git a/src/common/nallar/patched/storage/PatchChunk.java b/src/common/nallar/patched/storage/PatchChunk.java
@@ -216,9 +216,15 @@ public void addTileEntity(TileEntity tileEntity) {
 		}
 	}
 
-	@SuppressWarnings ("FieldRepeatedlyAccessedInMethod") // Patcher makes worldObj final
 	@Override
 	public void onChunkUnload() {
+		throw new Error("Not supported with TT");
+	}
+
+	@SuppressWarnings ("FieldRepeatedlyAccessedInMethod") // Patcher makes worldObj final
+	@Override
+	@Declare
+	public void onChunkUnloadTT() {
 		isChunkLoaded = false;
 		Set<TileEntity> removalSet = worldObj.tileEntityRemovalSet;
 		for (TileEntity var2 : (Iterable<TileEntity>) chunkTileEntityMap.values()) {

diff --git a/src/common/nallar/patched/storage/ThreadedChunkProvider.java b/src/common/nallar/patched/storage/ThreadedChunkProvider.java
@@ -154,24 +154,29 @@ public void tick() {
 				chunkCoordIntPair.chunkXPos = x;
 				chunkCoordIntPair.chunkZPos = z;
 				Chunk chunk = chunks.getValueByKey(key);
-				if (chunk == null || chunk.partiallyUnloaded || !chunk.queuedUnload) {
-					continue;
-				}
-				if (persistentChunks.containsKey(chunkCoordIntPair) || unloadingChunks.containsItem(key) || playerManager.getOrCreateChunkWatcher(x, z, false) != null || !fireBukkitUnloadEvent(chunk)) {
-					chunk.queuedUnload = false;
+				if (chunk == null) {
 					continue;
 				}
-				if (lastChunk == chunk) {
-					lastChunk = null;
-				}
-				chunk.partiallyUnloaded = true;
-				chunk.onChunkUnload();
-				chunk.pendingBlockUpdates = world.getPendingBlockUpdates(chunk, false);
-				loadedChunks.remove(chunk);
-				chunks.remove(key);
-				synchronized (unloadingChunks) {
-					unloadingChunks.put(key, chunk);
-					unloadStage1.add(new QueuedUnload(key, ticks));
+				synchronized (chunk) {
+					if (chunk.partiallyUnloaded || !chunk.queuedUnload) {
+						continue;
+					}
+					if (persistentChunks.containsKey(chunkCoordIntPair) || unloadingChunks.containsItem(key) || playerManager.getOrCreateChunkWatcher(x, z, false) != null || !fireBukkitUnloadEvent(chunk)) {
+						chunk.queuedUnload = false;
+						continue;
+					}
+					if (lastChunk == chunk) {
+						lastChunk = null;
+					}
+					chunk.partiallyUnloaded = true;
+					chunk.onChunkUnloadTT();
+					chunk.pendingBlockUpdates = world.getPendingBlockUpdates(chunk, false);
+					loadedChunks.remove(chunk);
+					chunks.remove(key);
+					synchronized (unloadingChunks) {
+						unloadingChunks.put(key, chunk);
+						unloadStage1.add(new QueuedUnload(key, ticks));
+					}
 				}
 			}
 
@@ -249,7 +254,7 @@ private boolean finalizeUnload(long key) {
 			try {
 				boolean notInUnload = !inUnload.getAndSet(true);
 				boolean notWorldGen = !worldGenInProgress.getAndSet(true);
-				safeSaveChunk(chunk);
+				saveChunk(chunk);
 				safeSaveExtraChunkData(chunk);
 				if (notWorldGen) {
 					worldGenInProgress.set(false);
@@ -357,20 +362,25 @@ public void unloadChunkImmediately(int x, int z, boolean save) {
 		long key = key(x, z);
 		finalizeUnload(key);
 		Chunk chunk = getChunkIfExists(x, z);
-		if (chunk == null || !fireBukkitUnloadEvent(chunk)) {
-			return;
-		}
-		synchronized (chunk) {
-			chunk.queuedUnload = true;
-			chunk.partiallyUnloaded = true;
-			chunk.onChunkUnload();
-			chunk.isChunkLoaded = false;
-			if (save || chunk.isModified) {
-				safeSaveChunk(chunk);
-				safeSaveExtraChunkData(chunk);
+		if (chunk != null) {
+			synchronized (chunk) {
+				if (chunk.partiallyUnloaded) {
+					return;
+				}
+				if (world.getPersistentChunks().containsKey(new ChunkCoordIntPair(x, z)) || world.getPlayerManager().getOrCreateChunkWatcher(x, z, false) != null || !fireBukkitUnloadEvent(chunk)) {
+					return;
+				}
+				chunk.queuedUnload = true;
+				chunk.partiallyUnloaded = true;
+				chunk.onChunkUnloadTT();
+				if (save || chunk.isModified) {
+					saveChunk(chunk);
+					safeSaveExtraChunkData(chunk);
+				}
+				chunk.alreadySavedAfterUnload = true;
+				loadedChunks.remove(chunk);
+				chunks.remove(key);
 			}
-			loadedChunks.remove(chunk);
-			chunks.remove(key);
 		}
 	}
 
@@ -730,8 +740,15 @@ protected void safeSaveExtraChunkData(Chunk chunk) {
 		}
 	}
 
+	@Deprecated
 	@Override
 	protected void safeSaveChunk(Chunk chunk) {
+		throw new Error("Not supported with TT");
+	}
+
+	@Override
+	@Declare
+	public void saveChunk(Chunk chunk) {
 		if (loader == null) {
 			return;
 		}
@@ -813,7 +830,7 @@ public boolean saveChunks(boolean fullSaveRequired, IProgressUpdate progressUpda
 				safeSaveExtraChunkData(chunk);
 			}
 
-			safeSaveChunk(chunk);
+			saveChunk(chunk);
 			chunk.isModified = false; // Just in case a mod is managing to set the chunk as modified during saving, don't want pointless repeated saving.
 		}
 

diff --git a/src/common/nallar/tickthreading/minecraft/TickThreading.java b/src/common/nallar/tickthreading/minecraft/TickThreading.java
@@ -46,6 +46,7 @@
 import nallar.tickthreading.util.ReflectUtil;
 import nallar.tickthreading.util.TableFormatter;
 import nallar.tickthreading.util.VersionUtil;
+import nallar.tickthreading.util.contextaccess.ContextAccess;
 import net.minecraft.command.ServerCommandManager;
 import net.minecraft.entity.EntityLiving;
 import net.minecraft.entity.item.EntityItem;
@@ -289,6 +290,9 @@ public synchronized void onWorldLoad(WorldEvent.Load event) {
 
 	@ForgeSubscribe
 	public synchronized void onWorldUnload(WorldEvent.Unload event) {
+		if (MinecraftServer.getServer().isServerRunning() && !ContextAccess.$.runningUnder(DimensionManager.class)) {
+			Log.severe("World unload event fired from unexpected location", new Throwable());
+		}
 		World world = event.world;
 		try {
 			TickManager tickManager = managers.remove(world);