SPIGOT-4849: Fix server crash when accessing chunks during chunk load/unload/populate events

This commit is contained in:
Irmo van den Berge 2019-07-13 20:19:44 +02:00 committed by md_5
parent f554183c3a
commit 989f9b3daa
3 changed files with 77 additions and 20 deletions

View file

@ -102,3 +102,22 @@
if (object2intmap.getInt(enumcreaturetype) <= k1) { if (object2intmap.getInt(enumcreaturetype) <= k1) {
SpawnerCreature.a(enumcreaturetype, (World) this.world, chunk, blockposition); SpawnerCreature.a(enumcreaturetype, (World) this.world, chunk, blockposition);
@@ -447,12 +489,18 @@
@Override
protected boolean executeNext() {
+ // CraftBukkit start - process pending Chunk loadCallback() and unloadCallback() after each run task
+ try {
if (ChunkProviderServer.this.tickDistanceManager()) {
return true;
} else {
ChunkProviderServer.this.lightEngine.queueUpdate();
return super.executeNext();
}
+ } finally {
+ playerChunkMap.callbackExecutor.run();
+ }
+ // CraftBukkit end
}
}
}

View file

@ -46,14 +46,14 @@
if (either == null || either.left().isPresent()) { if (either == null || either.left().isPresent()) {
return completablefuture; return completablefuture;
@@ -256,6 +265,21 @@ @@ -256,6 +265,24 @@
boolean flag1 = this.ticketLevel <= PlayerChunkMap.GOLDEN_TICKET; boolean flag1 = this.ticketLevel <= PlayerChunkMap.GOLDEN_TICKET;
PlayerChunk.State playerchunk_state = getChunkState(this.oldTicketLevel); PlayerChunk.State playerchunk_state = getChunkState(this.oldTicketLevel);
PlayerChunk.State playerchunk_state1 = getChunkState(this.ticketLevel); PlayerChunk.State playerchunk_state1 = getChunkState(this.ticketLevel);
+ // CraftBukkit start + // CraftBukkit start
+ // ChunkUnloadEvent: Called before the chunk is unloaded: isChunkLoaded is still true and chunk can still be modified by plugins. + // ChunkUnloadEvent: Called before the chunk is unloaded: isChunkLoaded is still true and chunk can still be modified by plugins.
+ if (playerchunk_state.isAtLeast(PlayerChunk.State.BORDER) && !playerchunk_state1.isAtLeast(PlayerChunk.State.BORDER)) { + if (playerchunk_state.isAtLeast(PlayerChunk.State.BORDER) && !playerchunk_state1.isAtLeast(PlayerChunk.State.BORDER)) {
+ this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAccept((either) -> { + this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAcceptAsync((either) -> {
+ either.ifLeft((chunkAccess) -> { + either.ifLeft((chunkAccess) -> {
+ Chunk chunk = (Chunk) chunkAccess; + Chunk chunk = (Chunk) chunkAccess;
+ // Minecraft will apply the chunks tick lists to the world once the chunk got loaded, and then store the tick + // Minecraft will apply the chunks tick lists to the world once the chunk got loaded, and then store the tick
@ -62,13 +62,16 @@
+ chunk.setNeedsSaving(true); + chunk.setNeedsSaving(true);
+ chunk.unloadCallback(); + chunk.unloadCallback();
+ }); + });
+ }); + }, playerchunkmap.callbackExecutor);
+
+ // Run callback right away if the future was already done
+ playerchunkmap.callbackExecutor.run();
+ } + }
+ // CraftBukkit end + // CraftBukkit end
CompletableFuture completablefuture; CompletableFuture completablefuture;
if (flag) { if (flag) {
@@ -287,7 +311,7 @@ @@ -287,7 +314,7 @@
if (flag2 && !flag3) { if (flag2 && !flag3) {
completablefuture = this.fullChunkFuture; completablefuture = this.fullChunkFuture;
this.fullChunkFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE; this.fullChunkFuture = PlayerChunk.UNLOADED_CHUNK_FUTURE;
@ -77,19 +80,22 @@
playerchunkmap.getClass(); playerchunkmap.getClass();
return either1.ifLeft(playerchunkmap::a); return either1.ifLeft(playerchunkmap::a);
})); }));
@@ -325,6 +349,17 @@ @@ -325,6 +352,20 @@
this.w.a(this.location, this::k, this.ticketLevel, this::d); this.w.a(this.location, this::k, this.ticketLevel, this::d);
this.oldTicketLevel = this.ticketLevel; this.oldTicketLevel = this.ticketLevel;
+ // CraftBukkit start + // CraftBukkit start
+ // ChunkLoadEvent: Called after the chunk is loaded: isChunkLoaded returns true and chunk is ready to be modified by plugins. + // ChunkLoadEvent: Called after the chunk is loaded: isChunkLoaded returns true and chunk is ready to be modified by plugins.
+ if (!playerchunk_state.isAtLeast(PlayerChunk.State.BORDER) && playerchunk_state1.isAtLeast(PlayerChunk.State.BORDER)) { + if (!playerchunk_state.isAtLeast(PlayerChunk.State.BORDER) && playerchunk_state1.isAtLeast(PlayerChunk.State.BORDER)) {
+ this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAccept((either) -> { + this.getStatusFutureUnchecked(ChunkStatus.FULL).thenAcceptAsync((either) -> {
+ either.ifLeft((chunkAccess) -> { + either.ifLeft((chunkAccess) -> {
+ Chunk chunk = (Chunk) chunkAccess; + Chunk chunk = (Chunk) chunkAccess;
+ chunk.loadCallback(); + chunk.loadCallback();
+ }); + });
+ }); + }, playerchunkmap.callbackExecutor);
+
+ // Run callback right away if the future was already done
+ playerchunkmap.callbackExecutor.run();
+ } + }
+ // CraftBukkit end + // CraftBukkit end
} }

View file

@ -17,7 +17,39 @@
private final AtomicInteger v; private final AtomicInteger v;
private final DefinedStructureManager definedStructureManager; private final DefinedStructureManager definedStructureManager;
private final File x; private final File x;
@@ -184,9 +185,12 @@ @@ -67,6 +68,31 @@
private final Queue<Runnable> A;
private int viewDistance;
+ // CraftBukkit start - recursion-safe executor for Chunk loadCallback() and unloadCallback()
+ public final CallbackExecutor callbackExecutor = new CallbackExecutor();
+ public static final class CallbackExecutor implements java.util.concurrent.Executor, Runnable {
+
+ private Runnable queued;
+
+ @Override
+ public void execute(Runnable runnable) {
+ if (queued != null) {
+ throw new IllegalStateException("Already queued");
+ }
+ queued = runnable;
+ }
+
+ @Override
+ public void run() {
+ Runnable task = queued;
+ queued = null;
+ if (task != null) {
+ task.run();
+ }
+ }
+ };
+ // CraftBukkit end
+
public PlayerChunkMap(WorldServer worldserver, File file, DataFixer datafixer, DefinedStructureManager definedstructuremanager, Executor executor, IAsyncTaskHandler<Runnable> iasynctaskhandler, ILightAccess ilightaccess, ChunkGenerator<?> chunkgenerator, WorldLoadListener worldloadlistener, Supplier<WorldPersistentData> supplier, int i) {
super(new File(worldserver.getWorldProvider().getDimensionManager().a(file), "region"), datafixer);
this.visibleChunks = this.updatingChunks.clone();
@@ -184,9 +210,12 @@
return completablefuture1.thenApply((list1) -> { return completablefuture1.thenApply((list1) -> {
List<IChunkAccess> list2 = Lists.newArrayList(); List<IChunkAccess> list2 = Lists.newArrayList();
@ -32,7 +64,7 @@
final Either<IChunkAccess, PlayerChunk.Failure> either = (Either) iterator.next(); final Either<IChunkAccess, PlayerChunk.Failure> either = (Either) iterator.next();
Optional<IChunkAccess> optional = either.left(); Optional<IChunkAccess> optional = either.left();
@@ -284,7 +288,7 @@ @@ -284,7 +313,7 @@
PlayerChunkMap.LOGGER.info("ThreadedAnvilChunkStorage ({}): All chunks are saved", this.x.getName()); PlayerChunkMap.LOGGER.info("ThreadedAnvilChunkStorage ({}): All chunks are saved", this.x.getName());
} else { } else {
this.visibleChunks.values().stream().filter(PlayerChunk::hasBeenLoaded).forEach((playerchunk) -> { this.visibleChunks.values().stream().filter(PlayerChunk::hasBeenLoaded).forEach((playerchunk) -> {
@ -41,7 +73,7 @@
if (ichunkaccess instanceof ProtoChunkExtension || ichunkaccess instanceof Chunk) { if (ichunkaccess instanceof ProtoChunkExtension || ichunkaccess instanceof Chunk) {
this.saveChunk(ichunkaccess); this.saveChunk(ichunkaccess);
@@ -295,7 +299,6 @@ @@ -295,7 +324,6 @@
} }
} }
@ -49,7 +81,7 @@
protected void unloadChunks(BooleanSupplier booleansupplier) { protected void unloadChunks(BooleanSupplier booleansupplier) {
GameProfilerFiller gameprofilerfiller = this.world.getMethodProfiler(); GameProfilerFiller gameprofilerfiller = this.world.getMethodProfiler();
@@ -334,7 +337,7 @@ @@ -334,7 +362,7 @@
private void a(long i, PlayerChunk playerchunk) { private void a(long i, PlayerChunk playerchunk) {
CompletableFuture<IChunkAccess> completablefuture = playerchunk.getChunkSave(); CompletableFuture<IChunkAccess> completablefuture = playerchunk.getChunkSave();
@ -58,7 +90,7 @@
CompletableFuture<IChunkAccess> completablefuture1 = playerchunk.getChunkSave(); CompletableFuture<IChunkAccess> completablefuture1 = playerchunk.getChunkSave();
if (completablefuture1 != completablefuture) { if (completablefuture1 != completablefuture) {
@@ -483,7 +486,7 @@ @@ -483,7 +511,7 @@
return CompletableFuture.completedFuture(Either.right(playerchunk_failure)); return CompletableFuture.completedFuture(Either.right(playerchunk_failure));
}); });
}, (runnable) -> { }, (runnable) -> {
@ -67,7 +99,7 @@
}); });
} }
@@ -564,7 +567,7 @@ @@ -564,7 +592,7 @@
long i = playerchunk.i().pair(); long i = playerchunk.i().pair();
playerchunk.getClass(); playerchunk.getClass();
@ -76,7 +108,7 @@
}); });
} }
@@ -581,7 +584,7 @@ @@ -581,7 +609,7 @@
return Either.left(chunk); return Either.left(chunk);
}); });
}, (runnable) -> { }, (runnable) -> {
@ -85,7 +117,7 @@
}); });
completablefuture1.thenAcceptAsync((either) -> { completablefuture1.thenAcceptAsync((either) -> {
@@ -595,7 +598,7 @@ @@ -595,7 +623,7 @@
return Either.left(chunk); return Either.left(chunk);
}); });
}, (runnable) -> { }, (runnable) -> {
@ -94,7 +126,7 @@
}); });
return completablefuture1; return completablefuture1;
} }
@@ -609,7 +612,7 @@ @@ -609,7 +637,7 @@
return chunk; return chunk;
}); });
}, (runnable) -> { }, (runnable) -> {
@ -103,7 +135,7 @@
}); });
} }
@@ -724,7 +727,7 @@ @@ -724,7 +752,7 @@
private NBTTagCompound readChunkData(ChunkCoordIntPair chunkcoordintpair) throws IOException { private NBTTagCompound readChunkData(ChunkCoordIntPair chunkcoordintpair) throws IOException {
NBTTagCompound nbttagcompound = this.read(chunkcoordintpair); NBTTagCompound nbttagcompound = this.read(chunkcoordintpair);
@ -112,7 +144,7 @@
} }
boolean isOutsideOfRange(ChunkCoordIntPair chunkcoordintpair) { boolean isOutsideOfRange(ChunkCoordIntPair chunkcoordintpair) {
@@ -1056,7 +1059,7 @@ @@ -1056,7 +1084,7 @@
public final Set<EntityPlayer> trackedPlayers = Sets.newHashSet(); public final Set<EntityPlayer> trackedPlayers = Sets.newHashSet();
public EntityTracker(Entity entity, int i, int j, boolean flag) { public EntityTracker(Entity entity, int i, int j, boolean flag) {
@ -121,7 +153,7 @@
this.tracker = entity; this.tracker = entity;
this.trackingDistance = i; this.trackingDistance = i;
this.e = SectionPosition.a(entity); this.e = SectionPosition.a(entity);
@@ -1109,7 +1112,7 @@ @@ -1109,7 +1137,7 @@
public void updatePlayer(EntityPlayer entityplayer) { public void updatePlayer(EntityPlayer entityplayer) {
if (entityplayer != this.tracker) { if (entityplayer != this.tracker) {
@ -130,7 +162,7 @@
int i = Math.min(this.trackingDistance, (PlayerChunkMap.this.viewDistance - 1) * 16); int i = Math.min(this.trackingDistance, (PlayerChunkMap.this.viewDistance - 1) * 16);
boolean flag = vec3d.x >= (double) (-i) && vec3d.x <= (double) i && vec3d.z >= (double) (-i) && vec3d.z <= (double) i && this.tracker.a(entityplayer); boolean flag = vec3d.x >= (double) (-i) && vec3d.x <= (double) i && vec3d.z >= (double) (-i) && vec3d.z <= (double) i && this.tracker.a(entityplayer);
@@ -1125,6 +1128,17 @@ @@ -1125,6 +1153,17 @@
} }
} }