Fix race condition with regionfile being closed right after getting one (#2812)

Occurs when 1 thread retrieves a regionfile, and then the
regionfile is closed due to it being thrown out of cache.
This commit is contained in:
Spottedleaf 2020-01-09 17:42:33 -08:00
parent fd6f3fb80f
commit 724664fc5f

View file

@ -121,7 +121,7 @@ tasks required to be executed by the chunk load task (i.e lighting
and some poi tasks).
diff --git a/src/main/java/co/aikar/timings/WorldTimingsHandler.java b/src/main/java/co/aikar/timings/WorldTimingsHandler.java
index 3a79cde595..8de6c4816c 100644
index 3a79cde59..8de6c4816 100644
--- a/src/main/java/co/aikar/timings/WorldTimingsHandler.java
+++ b/src/main/java/co/aikar/timings/WorldTimingsHandler.java
@@ -0,0 +0,0 @@ public class WorldTimingsHandler {
@ -161,7 +161,7 @@ index 3a79cde595..8de6c4816c 100644
public static Timing getTickList(WorldServer worldserver, String timingsType) {
diff --git a/src/main/java/com/destroystokyo/paper/PaperConfig.java b/src/main/java/com/destroystokyo/paper/PaperConfig.java
index 546a1cfe0a..1d7d1ffbf7 100644
index 546a1cfe0..1d7d1ffbf 100644
--- a/src/main/java/com/destroystokyo/paper/PaperConfig.java
+++ b/src/main/java/com/destroystokyo/paper/PaperConfig.java
@@ -0,0 +0,0 @@
@ -237,7 +237,7 @@ index 546a1cfe0a..1d7d1ffbf7 100644
+ }
}
diff --git a/src/main/java/com/destroystokyo/paper/antixray/ChunkPacketBlockControllerAntiXray.java b/src/main/java/com/destroystokyo/paper/antixray/ChunkPacketBlockControllerAntiXray.java
index 23626bef3a..1edcecd2ee 100644
index 23626bef3..1edcecd2e 100644
--- a/src/main/java/com/destroystokyo/paper/antixray/ChunkPacketBlockControllerAntiXray.java
+++ b/src/main/java/com/destroystokyo/paper/antixray/ChunkPacketBlockControllerAntiXray.java
@@ -0,0 +0,0 @@ import java.util.concurrent.Executors;
@ -318,7 +318,7 @@ index 23626bef3a..1edcecd2ee 100644
diff --git a/src/main/java/com/destroystokyo/paper/io/IOUtil.java b/src/main/java/com/destroystokyo/paper/io/IOUtil.java
new file mode 100644
index 0000000000..5af0ac3d9e
index 000000000..5af0ac3d9
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/io/IOUtil.java
@@ -0,0 +0,0 @@
@ -386,7 +386,7 @@ index 0000000000..5af0ac3d9e
+}
diff --git a/src/main/java/com/destroystokyo/paper/io/PaperFileIOThread.java b/src/main/java/com/destroystokyo/paper/io/PaperFileIOThread.java
new file mode 100644
index 0000000000..4f10a8311e
index 000000000..4f10a8311
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/io/PaperFileIOThread.java
@@ -0,0 +0,0 @@
@ -1053,7 +1053,7 @@ index 0000000000..4f10a8311e
+}
diff --git a/src/main/java/com/destroystokyo/paper/io/PrioritizedTaskQueue.java b/src/main/java/com/destroystokyo/paper/io/PrioritizedTaskQueue.java
new file mode 100644
index 0000000000..78bd238f4c
index 000000000..78bd238f4
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/io/PrioritizedTaskQueue.java
@@ -0,0 +0,0 @@
@ -1335,7 +1335,7 @@ index 0000000000..78bd238f4c
+}
diff --git a/src/main/java/com/destroystokyo/paper/io/QueueExecutorThread.java b/src/main/java/com/destroystokyo/paper/io/QueueExecutorThread.java
new file mode 100644
index 0000000000..ee906b594b
index 000000000..ee906b594
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/io/QueueExecutorThread.java
@@ -0,0 +0,0 @@
@ -1582,7 +1582,7 @@ index 0000000000..ee906b594b
+}
diff --git a/src/main/java/com/destroystokyo/paper/io/chunk/ChunkLoadTask.java b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkLoadTask.java
new file mode 100644
index 0000000000..305da47868
index 000000000..305da4786
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkLoadTask.java
@@ -0,0 +0,0 @@
@ -1737,7 +1737,7 @@ index 0000000000..305da47868
+}
diff --git a/src/main/java/com/destroystokyo/paper/io/chunk/ChunkSaveTask.java b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkSaveTask.java
new file mode 100644
index 0000000000..60312b85f9
index 000000000..60312b85f
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkSaveTask.java
@@ -0,0 +0,0 @@
@ -1855,7 +1855,7 @@ index 0000000000..60312b85f9
+}
diff --git a/src/main/java/com/destroystokyo/paper/io/chunk/ChunkTask.java b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkTask.java
new file mode 100644
index 0000000000..1dfa8abfd8
index 000000000..1dfa8abfd
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkTask.java
@@ -0,0 +0,0 @@
@ -1901,7 +1901,7 @@ index 0000000000..1dfa8abfd8
+}
diff --git a/src/main/java/com/destroystokyo/paper/io/chunk/ChunkTaskManager.java b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkTaskManager.java
new file mode 100644
index 0000000000..59d73bfad7
index 000000000..59d73bfad
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkTaskManager.java
@@ -0,0 +0,0 @@
@ -2359,7 +2359,7 @@ index 0000000000..59d73bfad7
+
+}
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index e9cd44fae1..1f6b1c4f16 100644
index e9cd44fae..1f6b1c4f1 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -0,0 +0,0 @@ public class ChunkProviderServer extends IChunkProvider {
@ -2529,7 +2529,7 @@ index e9cd44fae1..1f6b1c4f16 100644
} finally {
playerChunkMap.callbackExecutor.run();
diff --git a/src/main/java/net/minecraft/server/ChunkRegionLoader.java b/src/main/java/net/minecraft/server/ChunkRegionLoader.java
index a950ad801d..26f1a4b095 100644
index a950ad801..26f1a4b09 100644
--- a/src/main/java/net/minecraft/server/ChunkRegionLoader.java
+++ b/src/main/java/net/minecraft/server/ChunkRegionLoader.java
@@ -0,0 +0,0 @@ import it.unimi.dsi.fastutil.longs.LongOpenHashSet;
@ -2798,7 +2798,7 @@ index a950ad801d..26f1a4b095 100644
nbttagcompound1.set("PostProcessing", a(ichunkaccess.l()));
diff --git a/src/main/java/net/minecraft/server/ChunkStatus.java b/src/main/java/net/minecraft/server/ChunkStatus.java
index 134a4f0b7d..88f1674616 100644
index 134a4f0b7..88f167461 100644
--- a/src/main/java/net/minecraft/server/ChunkStatus.java
+++ b/src/main/java/net/minecraft/server/ChunkStatus.java
@@ -0,0 +0,0 @@ public class ChunkStatus {
@ -2810,7 +2810,7 @@ index 134a4f0b7d..88f1674616 100644
return ChunkStatus.r.getInt(chunkstatus.c());
}
diff --git a/src/main/java/net/minecraft/server/IAsyncTaskHandler.java b/src/main/java/net/minecraft/server/IAsyncTaskHandler.java
index 7210217913..f7156acb89 100644
index 721021791..f7156acb8 100644
--- a/src/main/java/net/minecraft/server/IAsyncTaskHandler.java
+++ b/src/main/java/net/minecraft/server/IAsyncTaskHandler.java
@@ -0,0 +0,0 @@ public abstract class IAsyncTaskHandler<R extends Runnable> implements Mailbox<R
@ -2823,7 +2823,7 @@ index 7210217913..f7156acb89 100644
;
}
diff --git a/src/main/java/net/minecraft/server/IChunkLoader.java b/src/main/java/net/minecraft/server/IChunkLoader.java
index 2f95174fcc..134c76065b 100644
index 2f95174fc..134c76065 100644
--- a/src/main/java/net/minecraft/server/IChunkLoader.java
+++ b/src/main/java/net/minecraft/server/IChunkLoader.java
@@ -0,0 +0,0 @@ package net.minecraft.server;
@ -2948,7 +2948,7 @@ index 2f95174fcc..134c76065b 100644
+// Paper end
}
diff --git a/src/main/java/net/minecraft/server/MCUtil.java b/src/main/java/net/minecraft/server/MCUtil.java
index 25a87c2d37..c02c53b50b 100644
index 25a87c2d3..c02c53b50 100644
--- a/src/main/java/net/minecraft/server/MCUtil.java
+++ b/src/main/java/net/minecraft/server/MCUtil.java
@@ -0,0 +0,0 @@ public final class MCUtil {
@ -2962,7 +2962,7 @@ index 25a87c2d37..c02c53b50b 100644
+ }
}
diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
index 4eba20b6ab..fe24d924e8 100644
index 4eba20b6a..fe24d924e 100644
--- a/src/main/java/net/minecraft/server/MinecraftServer.java
+++ b/src/main/java/net/minecraft/server/MinecraftServer.java
@@ -0,0 +0,0 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant<TickTas
@ -2974,7 +2974,7 @@ index 4eba20b6ab..fe24d924e8 100644
public String getServerIp() {
diff --git a/src/main/java/net/minecraft/server/NextTickListEntry.java b/src/main/java/net/minecraft/server/NextTickListEntry.java
index e9c405fb53..33cfeabdee 100644
index e9c405fb5..33cfeabde 100644
--- a/src/main/java/net/minecraft/server/NextTickListEntry.java
+++ b/src/main/java/net/minecraft/server/NextTickListEntry.java
@@ -0,0 +0,0 @@ import java.util.Comparator;
@ -2996,7 +2996,7 @@ index e9c405fb53..33cfeabdee 100644
this.e = t0;
this.b = i;
diff --git a/src/main/java/net/minecraft/server/NibbleArray.java b/src/main/java/net/minecraft/server/NibbleArray.java
index ed8c4a87b5..996c832638 100644
index ed8c4a87b..996c83263 100644
--- a/src/main/java/net/minecraft/server/NibbleArray.java
+++ b/src/main/java/net/minecraft/server/NibbleArray.java
@@ -0,0 +0,0 @@ public class NibbleArray {
@ -3008,7 +3008,7 @@ index ed8c4a87b5..996c832638 100644
return this.a == null ? new NibbleArray() : new NibbleArray((byte[]) this.a.clone());
}
diff --git a/src/main/java/net/minecraft/server/PlayerChunk.java b/src/main/java/net/minecraft/server/PlayerChunk.java
index 7a1578afaa..0fb9c1e441 100644
index 7a1578afa..0fb9c1e44 100644
--- a/src/main/java/net/minecraft/server/PlayerChunk.java
+++ b/src/main/java/net/minecraft/server/PlayerChunk.java
@@ -0,0 +0,0 @@ public class PlayerChunk {
@ -3034,7 +3034,7 @@ index 7a1578afaa..0fb9c1e441 100644
completablefuture = (CompletableFuture) this.statusFutures.get(i);
if (completablefuture != null) {
diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java
index 3825520fa1..d9faef8a67 100644
index 3825520fa..0ad11ba68 100644
--- a/src/main/java/net/minecraft/server/PlayerChunkMap.java
+++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java
@@ -0,0 +0,0 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@ -3433,9 +3433,11 @@ index 3825520fa1..d9faef8a67 100644
// Paper start - chunk status cache "api"
public ChunkStatus getChunkStatusOnDiskIfCached(ChunkCoordIntPair chunkPos) {
- RegionFile regionFile = this.getIOWorker().getRegionFileCache().getRegionFileIfLoaded(chunkPos);
+ synchronized (this) { // Paper
+ RegionFile regionFile = this.getRegionFileIfLoaded(chunkPos);
return regionFile == null ? null : regionFile.getStatusIfCached(chunkPos.x, chunkPos.z);
+ } // Paper
}
public ChunkStatus getChunkStatusOnDisk(ChunkCoordIntPair chunkPos) throws IOException {
@ -3542,9 +3544,18 @@ index 3825520fa1..d9faef8a67 100644
return this.m;
}
diff --git a/src/main/java/net/minecraft/server/RegionFile.java b/src/main/java/net/minecraft/server/RegionFile.java
index d37abf2cf3..0de24c792e 100644
index d37abf2cf..df728e2c0 100644
--- a/src/main/java/net/minecraft/server/RegionFile.java
+++ b/src/main/java/net/minecraft/server/RegionFile.java
@@ -0,0 +0,0 @@ public class RegionFile implements AutoCloseable {
private final RegionFileBitSet freeSectors;
public final File file;
+ public final java.util.concurrent.locks.ReentrantLock fileLock = new java.util.concurrent.locks.ReentrantLock(true); // Paper
+
// Paper start - Cache chunk status
private final ChunkStatus[] statuses = new ChunkStatus[32 * 32];
@@ -0,0 +0,0 @@ public class RegionFile implements AutoCloseable {
return (i + 4096 - 1) / 4096;
}
@ -3555,16 +3566,30 @@ index d37abf2cf3..0de24c792e 100644
if (i == 0) {
@@ -0,0 +0,0 @@ public class RegionFile implements AutoCloseable {
return chunkcoordintpair.j() + chunkcoordintpair.k() * 32;
}
- public void close() throws IOException {
+ public synchronized void close() throws IOException { // Paper - Synchronized
public void close() throws IOException {
+ // Paper start - Prevent regionfiles from being closed during use
+ this.fileLock.lock();
+ synchronized (this) {
+ try {
+ // Paper end
this.closed = true; // Paper
try {
this.c();
@@ -0,0 +0,0 @@ public class RegionFile implements AutoCloseable {
}
}
}
+ } finally { // Paper start - Prevent regionfiles from being closed during use
+ this.fileLock.unlock();
+ }
+ } // Paper end
}
diff --git a/src/main/java/net/minecraft/server/RegionFileCache.java b/src/main/java/net/minecraft/server/RegionFileCache.java
index e07ae98540..d927f93211 100644
index e07ae9854..0f201000f 100644
--- a/src/main/java/net/minecraft/server/RegionFileCache.java
+++ b/src/main/java/net/minecraft/server/RegionFileCache.java
@@ -0,0 +0,0 @@ import java.io.File;
@ -3588,11 +3613,70 @@ index e07ae98540..d927f93211 100644
// Paper end
- public RegionFile getFile(ChunkCoordIntPair chunkcoordintpair, boolean existingOnly) throws IOException { // CraftBukkit // Paper - private > public
+ public synchronized RegionFile getFile(ChunkCoordIntPair chunkcoordintpair, boolean existingOnly) throws IOException { // CraftBukkit // Paper - private > public, synchronize
+ // Paper start - add lock parameter
+ return this.getFile(chunkcoordintpair, existingOnly, false);
+ }
+ public synchronized RegionFile getFile(ChunkCoordIntPair chunkcoordintpair, boolean existingOnly, boolean lock) throws IOException {
+ // Paper end
long i = ChunkCoordIntPair.pair(chunkcoordintpair.getRegionX(), chunkcoordintpair.getRegionZ());
RegionFile regionfile = (RegionFile) this.cache.getAndMoveToFirst(i);
if (regionfile != null) {
+ // Paper start
+ if (lock) {
+ // must be in this synchronized block
+ regionfile.fileLock.lock();
+ }
+ // Paper end
return regionfile;
} else {
if (this.cache.size() >= com.destroystokyo.paper.PaperConfig.regionFileCacheSize) { // Paper - configurable
@@ -0,0 +0,0 @@ public final class RegionFileCache implements AutoCloseable {
RegionFile regionfile1 = new RegionFile(file, this.b);
this.cache.putAndMoveToFirst(i, regionfile1);
+ // Paper start
+ if (lock) {
+ // must be in this synchronized block
+ regionfile1.fileLock.lock();
+ }
+ // Paper end
return regionfile1;
}
}
@@ -0,0 +0,0 @@ public final class RegionFileCache implements AutoCloseable {
@Nullable
public NBTTagCompound read(ChunkCoordIntPair chunkcoordintpair) throws IOException {
- RegionFile regionfile = this.getFile(chunkcoordintpair, false); // CraftBukkit
+ RegionFile regionfile = this.getFile(chunkcoordintpair, false, true); // CraftBukkit // Paper
+ try { // Paper
DataInputStream datainputstream = regionfile.a(chunkcoordintpair);
// Paper start
if (regionfile.isOversized(chunkcoordintpair.x, chunkcoordintpair.z)) {
@@ -0,0 +0,0 @@ public final class RegionFileCache implements AutoCloseable {
}
return nbttagcompound;
+ } finally { // Paper start
+ regionfile.fileLock.unlock();
+ } // Paper end
}
protected void write(ChunkCoordIntPair chunkcoordintpair, NBTTagCompound nbttagcompound) throws IOException {
- RegionFile regionfile = this.getFile(chunkcoordintpair, false); // CraftBukkit
+ RegionFile regionfile = this.getFile(chunkcoordintpair, false, true); // CraftBukkit // Paper
+ try { // Paper
int attempts = 0; Exception laste = null; while (attempts++ < 5) { try { // Paper
DataOutputStream dataoutputstream = regionfile.c(chunkcoordintpair);
Throwable throwable = null;
@@ -0,0 +0,0 @@ public final class RegionFileCache implements AutoCloseable {
MinecraftServer.LOGGER.error("Failed to save chunk", laste);
}
// Paper end
+ } finally { // Paper start
+ regionfile.fileLock.unlock();
+ } // Paper end
}
- public void close() throws IOException {
@ -3610,7 +3694,7 @@ index e07ae98540..d927f93211 100644
return regionfile != null ? regionfile.chunkExists(pos) : false;
diff --git a/src/main/java/net/minecraft/server/RegionFileSection.java b/src/main/java/net/minecraft/server/RegionFileSection.java
index db9f0196bd..a6d8ef5eb4 100644
index db9f0196b..a6d8ef5eb 100644
--- a/src/main/java/net/minecraft/server/RegionFileSection.java
+++ b/src/main/java/net/minecraft/server/RegionFileSection.java
@@ -0,0 +0,0 @@ import javax.annotation.Nullable;
@ -3746,7 +3830,7 @@ index db9f0196bd..a6d8ef5eb4 100644
+ // Paper end
}
diff --git a/src/main/java/net/minecraft/server/TicketType.java b/src/main/java/net/minecraft/server/TicketType.java
index 335b644351..481d954808 100644
index 335b64435..481d95480 100644
--- a/src/main/java/net/minecraft/server/TicketType.java
+++ b/src/main/java/net/minecraft/server/TicketType.java
@@ -0,0 +0,0 @@ public class TicketType<T> {
@ -3758,7 +3842,7 @@ index 335b644351..481d954808 100644
public static <T> TicketType<T> a(String s, Comparator<T> comparator) {
return new TicketType<>(s, comparator, 0L);
diff --git a/src/main/java/net/minecraft/server/VillagePlace.java b/src/main/java/net/minecraft/server/VillagePlace.java
index c999f8c9bf..b59ef1a633 100644
index c999f8c9b..b59ef1a63 100644
--- a/src/main/java/net/minecraft/server/VillagePlace.java
+++ b/src/main/java/net/minecraft/server/VillagePlace.java
@@ -0,0 +0,0 @@ public class VillagePlace extends RegionFileSection<VillagePlaceSection> {
@ -3847,7 +3931,7 @@ index c999f8c9bf..b59ef1a633 100644
HAS_SPACE(VillagePlaceRecord::d), IS_OCCUPIED(VillagePlaceRecord::e), ANY((villageplacerecord) -> {
diff --git a/src/main/java/net/minecraft/server/WorldServer.java b/src/main/java/net/minecraft/server/WorldServer.java
index 90272d0c00..136c18d10b 100644
index 90272d0c0..136c18d10 100644
--- a/src/main/java/net/minecraft/server/WorldServer.java
+++ b/src/main/java/net/minecraft/server/WorldServer.java
@@ -0,0 +0,0 @@ public class WorldServer extends World {
@ -3940,7 +4024,7 @@ index 90272d0c00..136c18d10b 100644
// CraftBukkit start
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
index 2a33b90efd..0d036e5d96 100644
index 2a33b90ef..0d036e5d9 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
@@ -0,0 +0,0 @@ public class CraftWorld implements World {
@ -4002,7 +4086,7 @@ index 2a33b90efd..0d036e5d96 100644
@Override
public int getViewDistance() {
diff --git a/src/main/java/org/spigotmc/WatchdogThread.java b/src/main/java/org/spigotmc/WatchdogThread.java
index 07936eeba2..fe68df45ba 100644
index 07936eeba..fe68df45b 100644
--- a/src/main/java/org/spigotmc/WatchdogThread.java
+++ b/src/main/java/org/spigotmc/WatchdogThread.java
@@ -0,0 +0,0 @@ import java.lang.management.ThreadInfo;