From 3fe3746012d5f3575a063425b972f3aef47f8d71 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Sun, 20 Oct 2019 00:50:26 -0700 Subject: [PATCH] SPIGOT-5378: Fix TileEntity fixer deadlock Chunk loading logic can make getTileEntity calls, and these can be off of the main thread (i.e lighting). The TileEntity fixer makes a getType call, which will block on chunk load. Thus a deadlock can occur between a lighting thread and the server thread. --- nms-patches/World.patch | 9 +++++ nms-patches/WorldServer.patch | 62 +++++++++++++++++++---------------- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/nms-patches/World.patch b/nms-patches/World.patch index 1af8ae4075..8e7d1a7a67 100644 --- a/nms-patches/World.patch +++ b/nms-patches/World.patch @@ -23,6 +23,15 @@ public abstract class World implements IIBlockAccess, GeneratorAccess, AutoCloseable { protected static final Logger LOGGER = LogManager.getLogger(); +@@ -23,7 +39,7 @@ + protected final List tileEntityListPending = Lists.newArrayList(); + protected final List tileEntityListUnload = Lists.newArrayList(); + private final long b = 16777215L; +- private final Thread serverThread; ++ final Thread serverThread; // CraftBukkit - package private + private int u; + protected int i = (new Random()).nextInt(); + protected final int j = 1013904223; @@ -41,7 +57,51 @@ protected boolean tickingTileEntities; private final WorldBorder worldBorder; diff --git a/nms-patches/WorldServer.patch b/nms-patches/WorldServer.patch index c5e3c91b74..d4928da948 100644 --- a/nms-patches/WorldServer.patch +++ b/nms-patches/WorldServer.patch @@ -50,7 +50,7 @@ this.nextTickListBlock = new TickListServer<>(this, (block) -> { return block == null || block.getBlockData().isAir(); }, IRegistry.BLOCK::getKey, IRegistry.BLOCK::get, this::b); -@@ -85,9 +112,40 @@ +@@ -85,9 +112,44 @@ this.getWorldData().setGameType(minecraftserver.getGamemode()); } @@ -63,6 +63,10 @@ + @Override + public TileEntity getTileEntity(BlockPosition pos) { + TileEntity result = super.getTileEntity(pos); ++ if (Thread.currentThread() != this.serverThread) { ++ // SPIGOT-5378: avoid deadlock, this can be called in loading logic (i.e lighting) but getType() will block on chunk load ++ return result; ++ } + Block type = getType(pos).getBlock(); + + if (result != null && type != Blocks.AIR) { @@ -92,7 +96,7 @@ public void doTick(BooleanSupplier booleansupplier) { GameProfilerFiller gameprofilerfiller = this.getMethodProfiler(); -@@ -162,6 +220,7 @@ +@@ -162,6 +224,7 @@ this.rainLevel = MathHelper.a(this.rainLevel, 0.0F, 1.0F); } @@ -100,7 +104,7 @@ if (this.lastRainLevel != this.rainLevel) { this.server.getPlayerList().a((Packet) (new PacketPlayOutGameStateChange(7, this.rainLevel)), this.worldProvider.getDimensionManager()); } -@@ -180,13 +239,34 @@ +@@ -180,13 +243,34 @@ this.server.getPlayerList().sendAll(new PacketPlayOutGameStateChange(7, this.rainLevel)); this.server.getPlayerList().sendAll(new PacketPlayOutGameStateChange(8, this.thunderLevel)); } @@ -136,7 +140,7 @@ })) { this.C = false; if (this.getGameRules().getBoolean(GameRules.DO_DAYLIGHT_CYCLE)) { -@@ -225,7 +305,7 @@ +@@ -225,7 +309,7 @@ this.ae(); this.ticking = false; gameprofilerfiller.exitEnter("entities"); @@ -145,7 +149,7 @@ if (flag3) { this.resetEmptyTime(); -@@ -239,6 +319,11 @@ +@@ -239,6 +323,11 @@ for (i = 0; i < this.globalEntityList.size(); ++i) { entity = (Entity) this.globalEntityList.get(i); @@ -157,7 +161,7 @@ this.a((entity1) -> { ++entity1.ticksLived; entity1.tick(); -@@ -257,6 +342,7 @@ +@@ -257,6 +346,7 @@ Entity entity1 = (Entity) entry.getValue(); Entity entity2 = entity1.getVehicle(); @@ -165,7 +169,7 @@ if (!this.server.getSpawnAnimals() && (entity1 instanceof EntityAnimal || entity1 instanceof EntityWaterAnimal)) { entity1.die(); } -@@ -264,6 +350,7 @@ +@@ -264,6 +354,7 @@ if (!this.server.getSpawnNPCs() && entity1 instanceof NPC) { entity1.die(); } @@ -173,7 +177,7 @@ if (entity2 != null) { if (!entity2.dead && entity2.w(entity1)) { -@@ -324,10 +411,10 @@ +@@ -324,10 +415,10 @@ entityhorseskeleton.r(true); entityhorseskeleton.setAgeRaw(0); entityhorseskeleton.setPosition((double) blockposition.getX(), (double) blockposition.getY(), (double) blockposition.getZ()); @@ -186,7 +190,7 @@ } } -@@ -338,11 +425,11 @@ +@@ -338,11 +429,11 @@ BiomeBase biomebase = this.getBiome(blockposition); if (biomebase.a((IWorldReader) this, blockposition1)) { @@ -200,7 +204,7 @@ } if (flag && this.getBiome(blockposition1).b() == BiomeBase.Precipitation.RAIN) { -@@ -389,7 +476,7 @@ +@@ -389,7 +480,7 @@ protected BlockPosition a(BlockPosition blockposition) { BlockPosition blockposition1 = this.getHighestBlockYAt(HeightMap.Type.MOTION_BLOCKING, blockposition); AxisAlignedBB axisalignedbb = (new AxisAlignedBB(blockposition1, new BlockPosition(blockposition1.getX(), this.getBuildHeight(), blockposition1.getZ()))).g(3.0D); @@ -209,7 +213,7 @@ return entityliving != null && entityliving.isAlive() && this.f(entityliving.getChunkCoordinates()); }); -@@ -418,7 +505,7 @@ +@@ -418,7 +509,7 @@ while (iterator.hasNext()) { EntityPlayer entityplayer = (EntityPlayer) iterator.next(); @@ -218,7 +222,7 @@ ++i; } else if (entityplayer.isSleeping()) { ++j; -@@ -436,10 +523,22 @@ +@@ -436,10 +527,22 @@ } private void clearWeather() { @@ -243,7 +247,7 @@ } public void resetEmptyTime() { -@@ -477,6 +576,7 @@ +@@ -477,6 +580,7 @@ return IRegistry.ENTITY_TYPE.getKey(entity.getEntityType()).toString(); }); entity.tick(); @@ -251,7 +255,7 @@ this.getMethodProfiler().exit(); } -@@ -562,6 +662,22 @@ +@@ -562,6 +666,22 @@ BlockPosition blockposition = worldchunkmanager.a(0, 0, 256, list, random); ChunkCoordIntPair chunkcoordintpair = blockposition == null ? new ChunkCoordIntPair(0, 0) : new ChunkCoordIntPair(blockposition); @@ -274,7 +278,7 @@ if (blockposition == null) { WorldServer.LOGGER.warn("Unable to find spawn biome"); } -@@ -637,6 +753,7 @@ +@@ -637,6 +757,7 @@ ChunkProviderServer chunkproviderserver = this.getChunkProvider(); if (!flag1) { @@ -282,7 +286,7 @@ if (iprogressupdate != null) { iprogressupdate.a(new ChatMessage("menu.savingLevel", new Object[0])); } -@@ -648,6 +765,16 @@ +@@ -648,6 +769,16 @@ chunkproviderserver.save(flag); } @@ -299,7 +303,7 @@ } protected void k_() throws ExceptionWorldConflict { -@@ -719,7 +846,8 @@ +@@ -719,7 +850,8 @@ if (entity instanceof EntityInsentient) { EntityInsentient entityinsentient = (EntityInsentient) entity; @@ -309,7 +313,7 @@ continue; } } -@@ -736,11 +864,24 @@ +@@ -736,11 +868,24 @@ @Override public boolean addEntity(Entity entity) { @@ -336,7 +340,7 @@ } public void addEntityTeleport(Entity entity) { -@@ -790,13 +931,18 @@ +@@ -790,13 +935,18 @@ this.registerEntity(entityplayer); } @@ -357,7 +361,7 @@ IChunkAccess ichunkaccess = this.getChunkAt(MathHelper.floor(entity.locX / 16.0D), MathHelper.floor(entity.locZ / 16.0D), ChunkStatus.FULL, entity.attachedToPlayer); if (!(ichunkaccess instanceof Chunk)) { -@@ -824,7 +970,7 @@ +@@ -824,7 +974,7 @@ if (entity1 == null) { return false; } else { @@ -366,7 +370,7 @@ return true; } } -@@ -875,10 +1021,17 @@ +@@ -875,10 +1025,17 @@ } this.getScoreboard().a(entity); @@ -384,7 +388,7 @@ } private void registerEntity(Entity entity) { -@@ -899,9 +1052,16 @@ +@@ -899,9 +1056,16 @@ this.entitiesByUUID.put(entity.getUniqueID(), entity); this.getChunkProvider().addEntity(entity); @@ -401,7 +405,7 @@ } } -@@ -932,6 +1092,18 @@ +@@ -932,6 +1096,18 @@ } public void strikeLightning(EntityLightning entitylightning) { @@ -420,7 +424,7 @@ this.globalEntityList.add(entitylightning); this.server.getPlayerList().sendPacketNearby((EntityHuman) null, entitylightning.locX, entitylightning.locY, entitylightning.locZ, 512.0D, this.worldProvider.getDimensionManager(), new PacketPlayOutSpawnEntityWeather(entitylightning)); } -@@ -940,6 +1112,12 @@ +@@ -940,6 +1116,12 @@ public void a(int i, BlockPosition blockposition, int j) { Iterator iterator = this.server.getPlayerList().getPlayers().iterator(); @@ -433,7 +437,7 @@ while (iterator.hasNext()) { EntityPlayer entityplayer = (EntityPlayer) iterator.next(); -@@ -948,6 +1126,12 @@ +@@ -948,6 +1130,12 @@ double d1 = (double) blockposition.getY() - entityplayer.locY; double d2 = (double) blockposition.getZ() - entityplayer.locZ; @@ -446,7 +450,7 @@ if (d0 * d0 + d1 * d1 + d2 * d2 < 1024.0D) { entityplayer.playerConnection.sendPacket(new PacketPlayOutBlockBreakAnimation(i, blockposition, j)); } -@@ -1008,6 +1192,14 @@ +@@ -1008,6 +1196,14 @@ @Override public Explosion createExplosion(@Nullable Entity entity, DamageSource damagesource, double d0, double d1, double d2, float f, boolean flag, Explosion.Effect explosion_effect) { @@ -461,7 +465,7 @@ Explosion explosion = new Explosion(this, entity, d0, d1, d2, f, flag, explosion_effect); if (damagesource != null) { -@@ -1016,6 +1208,8 @@ +@@ -1016,6 +1212,8 @@ explosion.a(); explosion.a(false); @@ -470,7 +474,7 @@ if (explosion_effect == Explosion.Effect.NONE) { explosion.clearBlocks(); } -@@ -1080,13 +1274,20 @@ +@@ -1080,13 +1278,20 @@ } public int a(T t0, double d0, double d1, double d2, int i, double d3, double d4, double d5, double d6) { @@ -493,7 +497,7 @@ ++j; } } -@@ -1169,7 +1370,13 @@ +@@ -1169,7 +1374,13 @@ @Override public WorldMap a(String s) { return (WorldMap) this.getMinecraftServer().getWorldServer(DimensionManager.OVERWORLD).getWorldPersistentData().b(() -> {