Implement lock striping pattern for SpawnerData to eliminate lock contention

core/src/main/java/github/nighter/smartspawner/extras/HopperHandler.java

@@ -30,7 +30,6 @@ public class HopperHandler implements Listener {
     private final Map<Location, Scheduler.Task> activeHoppers = new ConcurrentHashMap<>();
     private final SpawnerManager spawnerManager;
     private final SpawnerGuiViewManager spawnerGuiViewManager;
-    private final Map<String, ReentrantLock> spawnerLocks = new ConcurrentHashMap<>();
 
     public HopperHandler(SmartSpawner plugin) {
         this.plugin = plugin;
@@ -113,7 +112,6 @@ public void onChunkUnload(ChunkUnloadEvent event) {
     public void cleanup() {
         activeHoppers.values().forEach(Scheduler.Task::cancel);
         activeHoppers.clear();
-        spawnerLocks.clear();
     }
 
     @EventHandler
@@ -134,10 +132,6 @@ public void onHopperBreak(BlockBreakEvent event) {
         }
     }
 
-    private ReentrantLock getOrCreateLock(SpawnerData spawner) {
-        return spawnerLocks.computeIfAbsent(spawner.getSpawnerId(), k -> new ReentrantLock());
-    }
-
     public void startHopperTask(Location hopperLoc, Location spawnerLoc) {
         if (!plugin.getConfig().getBoolean("hopper.enabled", false)) return;
         if (activeHoppers.containsKey(hopperLoc)) return;
@@ -191,7 +185,8 @@ private void transferItems(Location hopperLoc, Location spawnerLoc) {
         SpawnerData spawner = spawnerManager.getSpawnerByLocation(spawnerLoc);
         if (spawner == null) return;
 
-        ReentrantLock lock = getOrCreateLock(spawner);
+        // Use inventoryLock for hopper transfers
+        ReentrantLock lock = spawner.getInventoryLock();
         if (!lock.tryLock()) return; // Skip this tick if we can't get the lock
 
         try {

core/src/main/java/github/nighter/smartspawner/spawner/gui/synchronization/SpawnerGuiViewManager.java

@@ -1349,8 +1349,8 @@ private long calculateTimeUntilNextSpawn(SpawnerData spawner) {
         // If the timer has expired, handle spawn timing
         if (timeUntilNextSpawn <= 0) {
             try {
-                // Try to acquire lock with timeout to prevent deadlock
-                if (spawner.getLock().tryLock(100, TimeUnit.MILLISECONDS)) {
+                // Try to acquire dataLock with timeout to prevent deadlock
+                if (spawner.getDataLock().tryLock(100, TimeUnit.MILLISECONDS)) {
                     try {
                         // Re-check timing after acquiring lock
                         currentTime = System.currentTimeMillis();
@@ -1374,7 +1374,7 @@ private long calculateTimeUntilNextSpawn(SpawnerData spawner) {
 
                         return cachedDelay - timeElapsed;
                     } finally {
-                        spawner.getLock().unlock();
+                        spawner.getDataLock().unlock();
                     }
                 } else {
                     // If can't acquire lock, return minimal time to try again soon

core/src/main/java/github/nighter/smartspawner/spawner/lootgen/SpawnerLootGenerator.java

@@ -97,41 +97,69 @@ public LootResult generateLoot(int minMobs, int maxMobs, SpawnerData spawner) {
     public void spawnLootToSpawner(SpawnerData spawner) {
         // Try to acquire the lock, but don't block if it's already locked
         // This ensures we don't block the server thread while waiting for the lock
-        boolean lockAcquired = spawner.getLock().tryLock();
+        boolean lockAcquired = spawner.getLootGenerationLock().tryLock();
         if (!lockAcquired) {
-            // Lock is already held, which means stack size change is happening
+            // Lock is already held, which means another loot generation is happening
             // Skip this loot generation cycle
             return;
         }
 
         try {
-            long currentTime = System.currentTimeMillis();
-            long lastSpawnTime = spawner.getLastSpawnTime();
-            long spawnDelay = spawner.getSpawnDelay();
-
-            if (currentTime - lastSpawnTime < spawnDelay) {
+            // Acquire dataLock to safely read spawn timing and configuration values
+            // Use tryLock with short timeout to avoid blocking
+            boolean dataLockAcquired = false;
+            try {
+                dataLockAcquired = spawner.getDataLock().tryLock(50, java.util.concurrent.TimeUnit.MILLISECONDS);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+                return;
+            }
+
+            if (!dataLockAcquired) {
+                // dataLock is held (likely stack size change), skip this cycle
                 return;
             }
+
+            // Declare variables outside the try block so they're accessible in the async lambda
+            final long currentTime = System.currentTimeMillis();
+            final long spawnTime;
+            final EntityType entityType;
+            final int minMobs;
+            final int maxMobs;
+            final String spawnerId;
+            final AtomicInteger usedSlots;
+            final AtomicInteger maxSlots;
+
+            try {
+                long lastSpawnTime = spawner.getLastSpawnTime();
+                long spawnDelay = spawner.getSpawnDelay();
+
+                if (currentTime - lastSpawnTime < spawnDelay) {
+                    return;
+                }
 
-            // Get exact inventory slot usage
-            AtomicInteger usedSlots = new AtomicInteger(spawner.getVirtualInventory().getUsedSlots());
-            AtomicInteger maxSlots = new AtomicInteger(spawner.getMaxSpawnerLootSlots());
+                // Get exact inventory slot usage
+                usedSlots = new AtomicInteger(spawner.getVirtualInventory().getUsedSlots());
+                maxSlots = new AtomicInteger(spawner.getMaxSpawnerLootSlots());
 
-            // Check if both inventory and exp are full, only then skip loot generation
-            if (usedSlots.get() >= maxSlots.get() && spawner.getSpawnerExp() >= spawner.getMaxStoredExp()) {
-                if (!spawner.getIsAtCapacity()) {
-                    spawner.setIsAtCapacity(true);
+                // Check if both inventory and exp are full, only then skip loot generation
+                if (usedSlots.get() >= maxSlots.get() && spawner.getSpawnerExp() >= spawner.getMaxStoredExp()) {
+                    if (!spawner.getIsAtCapacity()) {
+                        spawner.setIsAtCapacity(true);
+                    }
+                    return; // Skip generation if both exp and inventory are full
                 }
-                return; // Skip generation if both exp and inventory are full
-            }
 
-            // Important: Store the current values we need for async processing
-            final EntityType entityType = spawner.getEntityType();
-            final int minMobs = spawner.getMinMobs();
-            final int maxMobs = spawner.getMaxMobs();
-            final String spawnerId = spawner.getSpawnerId();
-            // Store currentTime to update lastSpawnTime after successful loot addition
-            final long spawnTime = currentTime;
+                // Important: Store the current values we need for async processing
+                entityType = spawner.getEntityType();
+                minMobs = spawner.getMinMobs();
+                maxMobs = spawner.getMaxMobs();
+                spawnerId = spawner.getSpawnerId();
+                // Store currentTime to update lastSpawnTime after successful loot addition
+                spawnTime = currentTime;
+            } finally {
+                spawner.getDataLock().unlock();
+            }
 
             // Run heavy calculations async and batch updates using the Scheduler
             Scheduler.runTaskAsync(() -> {
@@ -148,7 +176,7 @@ public void spawnLootToSpawner(SpawnerData spawner) {
                     // Re-acquire the lock for the update phase
                     // This ensures the spawner hasn't been modified (like stack size changes)
                     // between our async calculations and now
-                    boolean updateLockAcquired = spawner.getLock().tryLock();
+                    boolean updateLockAcquired = spawner.getLootGenerationLock().tryLock();
                     if (!updateLockAcquired) {
                         // Lock is held, stack size is changing, skip this update
                         return;
@@ -198,7 +226,15 @@ public void spawnLootToSpawner(SpawnerData spawner) {
 
                         // Update spawn time only after successful loot addition
                         // This prevents skipped spawns when the lock fails
-                        spawner.setLastSpawnTime(spawnTime);
+                        // Must acquire dataLock to safely update lastSpawnTime
+                        boolean updateDataLockAcquired = spawner.getDataLock().tryLock();
+                        if (updateDataLockAcquired) {
+                            try {
+                                spawner.setLastSpawnTime(spawnTime);
+                            } finally {
+                                spawner.getDataLock().unlock();
+                            }
+                        }
 
                         // Check if spawner is now at capacity and update status if needed
                         spawner.updateCapacityStatus();
@@ -209,12 +245,12 @@ public void spawnLootToSpawner(SpawnerData spawner) {
                         // Mark for saving only once
                         spawnerManager.markSpawnerModified(spawner.getSpawnerId());
                     } finally {
-                        spawner.getLock().unlock();
+                        spawner.getLootGenerationLock().unlock();
                     }
                 });
             });
         } finally {
-            spawner.getLock().unlock();
+            spawner.getLootGenerationLock().unlock();
         }
     }
 

core/src/main/java/github/nighter/smartspawner/spawner/properties/SpawnerData.java

@@ -27,8 +27,16 @@ public class SpawnerData {
     private String spawnerId;
     @Getter
     private final Location spawnerLocation;
+
+    // Fine-grained locks for different operations (Lock Striping Pattern)
     @Getter
-    private final ReentrantLock lock = new ReentrantLock();
+    private final ReentrantLock inventoryLock = new ReentrantLock();  // For storage operations
+    @Getter
+    private final ReentrantLock lootGenerationLock = new ReentrantLock();  // For loot spawning
+    @Getter
+    private final ReentrantLock sellLock = new ReentrantLock();  // For selling operations
+    @Getter
+    private final ReentrantLock dataLock = new ReentrantLock();  // For metadata changes (exp, stack size, etc.)
 
     // Base values from config (immutable after load)
     private int baseMaxStoredExp;
@@ -185,11 +193,20 @@ private void createHologram() {
     }
 
     public void setStackSize(int stackSize) {
-        lock.lock();
+        // Acquire locks in consistent order to prevent deadlocks:
+        // 1. dataLock - for metadata changes
+        // 2. inventoryLock - to prevent inventory operations during virtual inventory replacement
+        // Note: We don't acquire lootGenerationLock here to avoid blocking loot generation cycles
+        dataLock.lock();
         try {
-            updateStackSize(stackSize);
+            inventoryLock.lock();
+            try {
+                updateStackSize(stackSize);
+            } finally {
+                inventoryLock.unlock();
+            }
         } finally {
-            lock.unlock();
+            dataLock.unlock();
         }
     }
 
@@ -215,6 +232,7 @@ private void updateStackSize(int newStackSize) {
         transferItemsToNewInventory(currentItems, newInventory);
         this.virtualInventory = newInventory;
 
+        // Reset lastSpawnTime to prevent exploit where players break spawners to trigger immediate loot
         this.lastSpawnTime = System.currentTimeMillis();
         updateHologramData();