diff --git a/sources/src/main/java/io/akarin/server/mixin/core/MixinWorldManager.java b/sources/src/main/java/io/akarin/server/mixin/core/MixinWorldManager.java index c79cab5f5..8105b54f9 100644 --- a/sources/src/main/java/io/akarin/server/mixin/core/MixinWorldManager.java +++ b/sources/src/main/java/io/akarin/server/mixin/core/MixinWorldManager.java @@ -16,9 +16,9 @@ public abstract class MixinWorldManager { @Overwrite public void a(Entity entity) { - this.world.getTracker().entriesLock.lock(); // Akarin + this.world.getTracker().entriesLock.writeLock().lock(); // Akarin this.world.getTracker().track(entity); - this.world.getTracker().entriesLock.unlock(); // Akarin + this.world.getTracker().entriesLock.writeLock().unlock(); // Akarin if (entity instanceof EntityPlayer) { this.world.worldProvider.a((EntityPlayer) entity); diff --git a/sources/src/main/java/net/minecraft/server/EntityPlayer.java b/sources/src/main/java/net/minecraft/server/EntityPlayer.java index e22dbfdea..244d15f6a 100644 --- a/sources/src/main/java/net/minecraft/server/EntityPlayer.java +++ b/sources/src/main/java/net/minecraft/server/EntityPlayer.java @@ -745,9 +745,9 @@ public class EntityPlayer extends EntityHuman implements ICrafting { if (entity instanceof EntityPlayer) { WorldServer worldServer = (WorldServer) entity.getWorld(); worldServer.tracker.untrackEntity(this); - worldServer.tracker.entriesLock.lock(); // Akarin + worldServer.tracker.entriesLock.writeLock().lock(); // Akarin - ProtocolSupport will overwrite track method worldServer.tracker.track(this); - worldServer.tracker.entriesLock.unlock(); // Akarin + worldServer.tracker.entriesLock.writeLock().unlock(); // Akarin - ProtocolSupport will overwrite track method } // Paper end diff --git a/sources/src/main/java/net/minecraft/server/EntityTracker.java b/sources/src/main/java/net/minecraft/server/EntityTracker.java index aeb5a8cd3..6e4501355 100644 --- a/sources/src/main/java/net/minecraft/server/EntityTracker.java +++ b/sources/src/main/java/net/minecraft/server/EntityTracker.java @@ -2,25 +2,24 @@ package net.minecraft.server; import com.google.common.collect.Lists; import com.google.common.collect.Sets; +import com.googlecode.concurentlocks.ReentrantReadWriteUpdateLock; import java.util.ArrayList; import java.util.Iterator; import java.util.Set; -import java.util.concurrent.locks.ReentrantLock; - import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; /** * Akarin Changes Note - * 1) Add lock for entries set operations (safety issue) + * 1) Made collections and entry access thread-safe (safety issue) */ public class EntityTracker { private static final Logger a = LogManager.getLogger(); private final WorldServer world; private final Set c = Sets.newHashSet(); - public final ReentrantLock entriesLock = new ReentrantLock(); // Akarin - add lock + public final ReentrantReadWriteUpdateLock entriesLock = new ReentrantReadWriteUpdateLock(); // Akarin - add lock public final IntHashMap trackedEntities = new IntHashMap(); private int e; @@ -38,7 +37,7 @@ public class EntityTracker { this.addEntity(entity, 512, 2); EntityPlayer entityplayer = (EntityPlayer) entity; Iterator iterator = this.c.iterator(); - entriesLock.lock(); // Akarin + // entriesLock.writeLock().lock(); // Akarin - locked in EntityPlayer while (iterator.hasNext()) { EntityTrackerEntry entitytrackerentry = (EntityTrackerEntry) iterator.next(); @@ -47,7 +46,7 @@ public class EntityTracker { entitytrackerentry.updatePlayer(entityplayer); } } - entriesLock.unlock(); // Akarin + // entriesLock.writeLock().unlock(); // Akarin - locked in EntityPlayer } else if (entity instanceof EntityFishingHook) { this.addEntity(entity, 64, 5, true); } else if (entity instanceof EntityArrow) { @@ -118,18 +117,18 @@ public class EntityTracker { org.spigotmc.AsyncCatcher.catchOp( "entity track"); // Spigot i = org.spigotmc.TrackingRange.getEntityTrackingRange(entity, i); // Spigot try { + // entriesLock.writeLock().lock(); // Akarin - locked from track method if (this.trackedEntities.b(entity.getId())) { throw new IllegalStateException("Entity is already tracked!"); } EntityTrackerEntry entitytrackerentry = new EntityTrackerEntry(entity, i, this.e, j, flag); - entriesLock.lock(); // Akarin this.c.add(entitytrackerentry); this.trackedEntities.a(entity.getId(), entitytrackerentry); entitytrackerentry.scanPlayers(this.world.players); - entriesLock.unlock(); // Akarin + // entriesLock.writeLock().unlock(); // Akarin - locked from track method } catch (Throwable throwable) { CrashReport crashreport = CrashReport.a(throwable, "Adding entity to track"); CrashReportSystemDetails crashreportsystemdetails = crashreport.a("Entity To Track"); @@ -166,34 +165,32 @@ public class EntityTracker { public void untrackEntity(Entity entity) { org.spigotmc.AsyncCatcher.catchOp( "entity untrack"); // Spigot + entriesLock.writeLock().lock(); // Akarin if (entity instanceof EntityPlayer) { EntityPlayer entityplayer = (EntityPlayer) entity; Iterator iterator = this.c.iterator(); - entriesLock.lock(); // Akarin while (iterator.hasNext()) { EntityTrackerEntry entitytrackerentry = (EntityTrackerEntry) iterator.next(); entitytrackerentry.a(entityplayer); } - entriesLock.unlock(); // Akarin } EntityTrackerEntry entitytrackerentry1 = this.trackedEntities.d(entity.getId()); if (entitytrackerentry1 != null) { - entriesLock.lock(); // Akarin this.c.remove(entitytrackerentry1); entitytrackerentry1.a(); - entriesLock.unlock(); // Akarin } + entriesLock.writeLock().unlock(); // Akarin } public void updatePlayers() { ArrayList arraylist = Lists.newArrayList(); Iterator iterator = this.c.iterator(); world.timings.tracker1.startTiming(); // Spigot - entriesLock.lock(); // Akarin + entriesLock.writeLock().lock(); // Akarin while (iterator.hasNext()) { EntityTrackerEntry entitytrackerentry = (EntityTrackerEntry) iterator.next(); @@ -221,14 +218,14 @@ public class EntityTracker { } } } - entriesLock.unlock(); // Akarin + entriesLock.writeLock().unlock(); // Akarin world.timings.tracker2.stopTiming(); // Spigot } public void a(EntityPlayer entityplayer) { Iterator iterator = this.c.iterator(); - entriesLock.lock(); // Akarin + entriesLock.writeLock().lock(); // Akarin while (iterator.hasNext()) { EntityTrackerEntry entitytrackerentry = (EntityTrackerEntry) iterator.next(); @@ -239,11 +236,13 @@ public class EntityTracker { entitytrackerentry.updatePlayer(entityplayer); } } - entriesLock.unlock(); // Akarin + entriesLock.writeLock().unlock(); // Akarin } public void a(Entity entity, Packet packet) { + entriesLock.readLock().lock(); // Akarin EntityTrackerEntry entitytrackerentry = this.trackedEntities.get(entity.getId()); + entriesLock.readLock().unlock(); // Akarin if (entitytrackerentry != null) { entitytrackerentry.broadcast(packet); @@ -252,7 +251,9 @@ public class EntityTracker { } public void sendPacketToEntity(Entity entity, Packet packet) { + entriesLock.readLock().lock(); // Akarin EntityTrackerEntry entitytrackerentry = this.trackedEntities.get(entity.getId()); + entriesLock.readLock().unlock(); // Akarin if (entitytrackerentry != null) { entitytrackerentry.broadcastIncludingSelf(packet); @@ -262,21 +263,21 @@ public class EntityTracker { public void untrackPlayer(EntityPlayer entityplayer) { Iterator iterator = this.c.iterator(); - entriesLock.lock(); // Akarin + entriesLock.writeLock().lock(); while (iterator.hasNext()) { EntityTrackerEntry entitytrackerentry = (EntityTrackerEntry) iterator.next(); entitytrackerentry.clear(entityplayer); } - entriesLock.unlock(); // Akarin + entriesLock.writeLock().unlock(); } public void a(EntityPlayer entityplayer, Chunk chunk) { ArrayList arraylist = Lists.newArrayList(); ArrayList arraylist1 = Lists.newArrayList(); Iterator iterator = this.c.iterator(); - entriesLock.lock(); // Akarin + entriesLock.writeLock().lock(); // Akarin while (iterator.hasNext()) { EntityTrackerEntry entitytrackerentry = (EntityTrackerEntry) iterator.next(); @@ -293,7 +294,7 @@ public class EntityTracker { } } } - entriesLock.unlock(); // Akarin + entriesLock.writeLock().unlock(); // Akarin Entity entity1; @@ -320,13 +321,13 @@ public class EntityTracker { public void a(int i) { this.e = (i - 1) * 16; Iterator iterator = this.c.iterator(); - entriesLock.lock(); // Akarin + entriesLock.readLock().lock(); // Akarin while (iterator.hasNext()) { EntityTrackerEntry entitytrackerentry = (EntityTrackerEntry) iterator.next(); entitytrackerentry.a(this.e); } - entriesLock.unlock(); // Akarin + entriesLock.readLock().unlock(); // Akarin } } diff --git a/sources/src/main/java/net/minecraft/server/EntityTrackerEntry.java b/sources/src/main/java/net/minecraft/server/EntityTrackerEntry.java index 6645c26ea..5e946cfca 100644 --- a/sources/src/main/java/net/minecraft/server/EntityTrackerEntry.java +++ b/sources/src/main/java/net/minecraft/server/EntityTrackerEntry.java @@ -15,14 +15,14 @@ import org.bukkit.event.player.PlayerVelocityEvent; /** * Akarin Changes Note - * 1) Make trackedPlayerMap thread-safe (safety issue) + * 1) Made entry access thread-safe (safety issue) */ public class EntityTrackerEntry { private static final Logger c = LogManager.getLogger(); private final Entity tracker; private final int e; - private int f; + private volatile int f; // Akarin private final int g; private long xLoc; private long yLoc; @@ -47,7 +47,7 @@ public class EntityTrackerEntry { // Paper start // Replace trackedPlayers Set with a Map. The value is true until the player receives // their first update (which is forced to have absolute coordinates), false afterward. - public java.util.Map trackedPlayerMap = new java.util.concurrent.ConcurrentHashMap(); // Akarin - make concurrent + public java.util.Map trackedPlayerMap = new java.util.HashMap(); public Set trackedPlayers = trackedPlayerMap.keySet(); // Paper end @@ -489,13 +489,23 @@ public class EntityTrackerEntry { return false; } private static boolean isTrackedBy(Entity entity, EntityPlayer entityplayer) { - return entity == entityplayer || entity.tracker != null && entity.tracker.trackedPlayers.contains(entityplayer); + // Akarin start + boolean mayTrackedBy = entity == entityplayer || entity.tracker != null; + if (mayTrackedBy) { + ((WorldServer) entity.world).tracker.entriesLock.readLock().lock(); + mayTrackedBy = entity.tracker.trackedPlayers.contains(entityplayer); + ((WorldServer) entity.world).tracker.entriesLock.readLock().unlock(); + } + return mayTrackedBy; + // Akarin end } private void updatePassengers(EntityPlayer player) { if (tracker.isVehicle()) { tracker.passengers.forEach((e) -> { if (e.tracker != null) { + ((WorldServer) e.world).tracker.entriesLock.writeLock().lock(); // Akarin e.tracker.updatePlayer(player); + ((WorldServer) e.world).tracker.entriesLock.writeLock().unlock(); // Akarin } }); player.playerConnection.sendPacket(new PacketPlayOutMount(this.tracker)); diff --git a/sources/src/main/java/net/minecraft/server/PlayerChunkMap.java b/sources/src/main/java/net/minecraft/server/PlayerChunkMap.java index fefd8dac8..d0001b133 100644 --- a/sources/src/main/java/net/minecraft/server/PlayerChunkMap.java +++ b/sources/src/main/java/net/minecraft/server/PlayerChunkMap.java @@ -234,11 +234,8 @@ public class PlayerChunkMap { } // Paper timing } - boolean unlockRequired = true; // Akarin managedPlayersLock.readLock().lock(); // Akarin if (this.managedPlayers.isEmpty()) { - managedPlayersLock.readLock().unlock(); // Akarin - unlockRequired = false; // Akarin try (Timing ignored = world.timings.doChunkMapUnloadChunks.startTiming()) { // Paper WorldProvider worldprovider = this.world.worldProvider; @@ -247,7 +244,7 @@ public class PlayerChunkMap { } } // Paper timing } - if (unlockRequired) managedPlayersLock.readLock().unlock(); // Akarin + managedPlayersLock.readLock().unlock(); // Akarin } diff --git a/sources/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/sources/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java index 9d562e4ea..e215bfae6 100644 --- a/sources/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java +++ b/sources/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java @@ -1107,10 +1107,14 @@ public class CraftPlayer extends CraftHumanEntity implements Player { EntityTracker tracker = ((WorldServer) entity.world).tracker; // Paper end + tracker.entriesLock.updateLock().lock(); // Akarin EntityTrackerEntry entry = tracker.trackedEntities.get(other.getId()); if (entry != null) { + tracker.entriesLock.writeLock().lock(); // Akarin entry.clear(getHandle()); + tracker.entriesLock.writeLock().unlock(); // Akarin } + tracker.entriesLock.updateLock().unlock(); // Akarin // Remove the hidden player from this player user list, if they're on it if (other.sentListPacket) { @@ -1157,12 +1161,14 @@ public class CraftPlayer extends CraftHumanEntity implements Player { getHandle().playerConnection.sendPacket(new PacketPlayOutPlayerInfo(PacketPlayOutPlayerInfo.EnumPlayerInfoAction.ADD_PLAYER, other)); - tracker.entriesLock.lock(); // Akarin + tracker.entriesLock.updateLock().lock(); // Akarin EntityTrackerEntry entry = tracker.trackedEntities.get(other.getId()); if (entry != null && !entry.trackedPlayers.contains(getHandle())) { + tracker.entriesLock.writeLock().lock(); // Akarin entry.updatePlayer(getHandle()); - tracker.entriesLock.unlock(); // Akarin + tracker.entriesLock.writeLock().unlock(); // Akarin } + tracker.entriesLock.updateLock().unlock(); // Akarin } // Paper start private void reregisterPlayer(EntityPlayer player) {