3
0
Mirror von https://github.com/PaperMC/Paper.git synchronisiert 2024-12-18 12:30:06 +01:00

Fix a few chunk saving race conditions

* ChunkRegionLoader.c() picks an entry in the save queue, removes that entry from the save queue and then actually writes the entry to the region file. So, between the last two steps, the entry is neither in the save queue nor is it in the region file; if somebody loads the chunk again (with ChunkRegionLoader.loadChunk()) in that gap, they'll get old data. I've delayed the removal until the saving is done.
* ChunkRegionLoader.c() also records the coords of the chunks it's currently saving in this.c. ChunkRegionLoader.a(ChunkCoordIntPair, NBTTagCompound), which adds an entry to the save queue, stops the addition of an entry if its coords are in this.c. Now, I'm guessing that Mojang's intended purpose for this mechanism was to prevent multiple parallel writes for the same chunk. The "stops the addition" bit above should then be something like "block until it's no longer in c"; in fact, the vanilla implementation is "discard the new state of the chunk". I've taken the easy route to solving this, by just making ChunkRegionLoader.c() synchronized (since, in normal use, only the chunk saving thread is in here).
Dieser Commit ist enthalten in:
Geoff Crossland 2017-08-11 17:27:33 +10:00 committet von md_5
Ursprung 27b8bf9116
Commit 963b19c599

Datei anzeigen

@ -1,6 +1,20 @@
--- a/net/minecraft/server/ChunkRegionLoader.java --- a/net/minecraft/server/ChunkRegionLoader.java
+++ b/net/minecraft/server/ChunkRegionLoader.java +++ b/net/minecraft/server/ChunkRegionLoader.java
@@ -29,19 +29,35 @@ @@ -19,29 +19,47 @@
private static final Logger a = LogManager.getLogger();
private final Map<ChunkCoordIntPair, NBTTagCompound> b = Maps.newConcurrentMap();
- private final Set<ChunkCoordIntPair> c = Collections.newSetFromMap(Maps.newConcurrentMap());
+ // CraftBukkit
+ // private final Set<ChunkCoordIntPair> c = Collections.newSetFromMap(Maps.newConcurrentMap());
private final File d;
private final DataConverterManager e;
- private boolean f;
+ // private boolean f;
+ // CraftBukkit
public ChunkRegionLoader(File file, DataConverterManager dataconvertermanager) {
this.d = file;
this.e = dataconvertermanager; this.e = dataconvertermanager;
} }
@ -39,7 +53,7 @@
} }
return this.a(world, i, j, nbttagcompound); return this.a(world, i, j, nbttagcompound);
@@ -55,7 +71,7 @@ @@ -55,7 +73,7 @@
} }
@Nullable @Nullable
@ -48,7 +62,7 @@
if (!nbttagcompound.hasKeyOfType("Level", 10)) { if (!nbttagcompound.hasKeyOfType("Level", 10)) {
ChunkRegionLoader.a.error("Chunk file at {},{} is missing level data, skipping", Integer.valueOf(i), Integer.valueOf(j)); ChunkRegionLoader.a.error("Chunk file at {},{} is missing level data, skipping", Integer.valueOf(i), Integer.valueOf(j));
return null; return null;
@@ -72,10 +88,28 @@ @@ -72,10 +90,28 @@
ChunkRegionLoader.a.error("Chunk file at {},{} is in the wrong location; relocating. (Expected {}, {}, got {}, {})", Integer.valueOf(i), Integer.valueOf(j), Integer.valueOf(i), Integer.valueOf(j), Integer.valueOf(chunk.locX), Integer.valueOf(chunk.locZ)); ChunkRegionLoader.a.error("Chunk file at {},{} is in the wrong location; relocating. (Expected {}, {}, got {}, {})", Integer.valueOf(i), Integer.valueOf(j), Integer.valueOf(i), Integer.valueOf(j), Integer.valueOf(chunk.locX), Integer.valueOf(chunk.locZ));
nbttagcompound1.setInt("xPos", i); nbttagcompound1.setInt("xPos", i);
nbttagcompound1.setInt("zPos", j); nbttagcompound1.setInt("zPos", j);
@ -78,16 +92,32 @@
} }
} }
} }
@@ -106,20 +140,27 @@ @@ -98,7 +134,9 @@
}
protected void a(ChunkCoordIntPair chunkcoordintpair, NBTTagCompound nbttagcompound) {
- if (!this.c.contains(chunkcoordintpair)) {
+ // CraftBukkit
+ // if (!this.c.contains(chunkcoordintpair))
+ {
this.b.put(chunkcoordintpair, nbttagcompound);
}
@@ -106,20 +144,32 @@
} }
public boolean a() { public boolean a() {
- if (this.b.isEmpty()) { - if (this.b.isEmpty()) {
- if (this.f) {
+ // CraftBukkit start + // CraftBukkit start
+ return this.processSaveQueueEntry(false);
+ }
+
+ private synchronized boolean processSaveQueueEntry(boolean logCompletion) {
+ Iterator<Map.Entry<ChunkCoordIntPair, NBTTagCompound>> iter = this.b.entrySet().iterator(); + Iterator<Map.Entry<ChunkCoordIntPair, NBTTagCompound>> iter = this.b.entrySet().iterator();
+ if (!iter.hasNext()) { + if (!iter.hasNext()) {
+ if (logCompletion) {
+ // CraftBukkit end + // CraftBukkit end
if (this.f) {
ChunkRegionLoader.a.info("ThreadedAnvilChunkStorage ({}): All chunks are saved", this.d.getName()); ChunkRegionLoader.a.info("ThreadedAnvilChunkStorage ({}): All chunks are saved", this.d.getName());
} }
@ -96,20 +126,31 @@
- ChunkCoordIntPair chunkcoordintpair = (ChunkCoordIntPair) this.b.keySet().iterator().next(); - ChunkCoordIntPair chunkcoordintpair = (ChunkCoordIntPair) this.b.keySet().iterator().next();
+ // CraftBukkit start + // CraftBukkit start
+ Map.Entry<ChunkCoordIntPair, NBTTagCompound> entry = iter.next(); + Map.Entry<ChunkCoordIntPair, NBTTagCompound> entry = iter.next();
+ iter.remove(); // Pop single entry
+ ChunkCoordIntPair chunkcoordintpair = entry.getKey(); + ChunkCoordIntPair chunkcoordintpair = entry.getKey();
+ NBTTagCompound nbttagcompound = entry.getValue();
+ // CraftBukkit end + // CraftBukkit end
boolean flag; boolean flag;
try { try {
this.c.add(chunkcoordintpair); - this.c.add(chunkcoordintpair);
- NBTTagCompound nbttagcompound = (NBTTagCompound) this.b.remove(chunkcoordintpair); - NBTTagCompound nbttagcompound = (NBTTagCompound) this.b.remove(chunkcoordintpair);
+ NBTTagCompound nbttagcompound = (NBTTagCompound) entry.getValue(); // CraftBukkit + // this.c.add(chunkcoordintpair);
+ // NBTTagCompound nbttagcompound = (NBTTagCompound) this.b.remove(chunkcoordintpair);
+ // CraftBukkit
if (nbttagcompound != null) { if (nbttagcompound != null) {
try { try {
@@ -139,10 +180,14 @@ @@ -131,7 +181,7 @@
flag = true;
} finally {
- this.c.remove(chunkcoordintpair);
+ this.b.remove(chunkcoordintpair, nbttagcompound); // CraftBukkit
}
return flag;
@@ -139,10 +189,14 @@
} }
private void b(ChunkCoordIntPair chunkcoordintpair, NBTTagCompound nbttagcompound) throws IOException { private void b(ChunkCoordIntPair chunkcoordintpair, NBTTagCompound nbttagcompound) throws IOException {
@ -125,15 +166,27 @@
} }
public void b(World world, Chunk chunk) throws IOException {} public void b(World world, Chunk chunk) throws IOException {}
@@ -157,6 +202,7 @@ @@ -151,15 +205,16 @@
if (this.a()) {
public void c() {
try {
- this.f = true;
+ // this.f = true; // CraftBukkit
while (true) {
- if (this.a()) {
+ if (this.processSaveQueueEntry(true)) { // CraftBukkit
continue; continue;
} }
+ break; // CraftBukkit - Fix infinite loop when saving chunks + break; // CraftBukkit - Fix infinite loop when saving chunks
} }
} finally { } finally {
this.f = false; - this.f = false;
@@ -334,6 +380,13 @@ + // this.f = false; // CraftBukkit
}
}
@@ -334,6 +389,13 @@
chunk.a(nbttagcompound.getByteArray("Biomes")); chunk.a(nbttagcompound.getByteArray("Biomes"));
} }
@ -147,7 +200,7 @@
NBTTagList nbttaglist1 = nbttagcompound.getList("Entities", 10); NBTTagList nbttaglist1 = nbttagcompound.getList("Entities", 10);
for (int l = 0; l < nbttaglist1.size(); ++l) { for (int l = 0; l < nbttaglist1.size(); ++l) {
@@ -371,7 +424,7 @@ @@ -371,7 +433,7 @@
} }
} }
@ -156,7 +209,7 @@
} }
@Nullable @Nullable
@@ -399,14 +452,20 @@ @@ -399,14 +461,20 @@
} }
@Nullable @Nullable
@ -178,7 +231,7 @@
return null; return null;
} else { } else {
if (nbttagcompound.hasKeyOfType("Passengers", 9)) { if (nbttagcompound.hasKeyOfType("Passengers", 9)) {
@@ -435,8 +494,14 @@ @@ -435,8 +503,14 @@
} }
} }