Better handle locks for EntityTracker and misc

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);

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
 

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<EntityTrackerEntry> c = Sets.newHashSet();
-    public final ReentrantLock entriesLock = new ReentrantLock(); // Akarin - add lock
+    public final ReentrantReadWriteUpdateLock entriesLock = new ReentrantReadWriteUpdateLock(); // Akarin - add lock
     public final IntHashMap<EntityTrackerEntry> 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
     }
 }

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<EntityPlayer, Boolean> trackedPlayerMap = new java.util.concurrent.ConcurrentHashMap<EntityPlayer, Boolean>(); // Akarin - make concurrent
+    public java.util.Map<EntityPlayer, Boolean> trackedPlayerMap = new java.util.HashMap<EntityPlayer, Boolean>();
     public Set<EntityPlayer> 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));

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
 
     }
 

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) {