From 02f75d0e7f82219348fc6b528e7561a26d352f53 Mon Sep 17 00:00:00 2001
From: Andreas Troelsen <garbagemule@gmail.com>
Date: Mon, 23 Apr 2018 00:51:14 +0200
Subject: [PATCH] Prevent infinite loops due to PlayerDropItemEvent from /give
 command.

Apparently, the /give command will drop an item "as" the target player, resulting in the player picking up the item if there is room in their inventory. This triggers a PlayerDropItemEvent, which MobArena catches and tries to cancel if the target player is in an arena. Furthermore, if the player is a spectator outside of the arena region, MobArena will force the player to leave the arena. This triggers the reward granting logic, so using the /give command as a reward has the potential of causing an infinite loop.

This commit introduces the idea of "leaving players", i.e. players in the process of leaving the arena. In the event of a dropped item from a player who is currently leaving an arena, the PlayerDropItemEvent is ignored, because it is assumed to be from the /give command. Note that this doesn't actually prevent normal PlayerDropItemEvents from causing a forced leave, since the player won't be in the process of leaving at that specific point.
---
 .../java/com/garbagemule/MobArena/ArenaImpl.java  | 15 +++++++++++++++
 .../com/garbagemule/MobArena/ArenaListener.java   | 15 +++++++++++++++
 .../com/garbagemule/MobArena/framework/Arena.java |  2 ++
 3 files changed, 32 insertions(+)

diff --git a/src/main/java/com/garbagemule/MobArena/ArenaImpl.java b/src/main/java/com/garbagemule/MobArena/ArenaImpl.java
index f8b6c44..c411983 100644
--- a/src/main/java/com/garbagemule/MobArena/ArenaImpl.java
+++ b/src/main/java/com/garbagemule/MobArena/ArenaImpl.java
@@ -97,6 +97,7 @@ public class ArenaImpl implements Arena
     private Map<Player,PlayerData> playerData = new HashMap<>();
     
     private Set<Player> arenaPlayers, lobbyPlayers, readyPlayers, specPlayers, deadPlayers;
+    private Set<Player> leavingPlayers;
     private Set<Player> randoms;
     
     // Classes stuff
@@ -164,6 +165,7 @@ public class ArenaImpl implements Arena
         this.specPlayers    = new HashSet<>();
         this.deadPlayers    = new HashSet<>();
         this.randoms        = new HashSet<>();
+        this.leavingPlayers = new HashSet<>();
 
         // Classes, items and permissions
         this.classes      = plugin.getArenaMaster().getClasses();
@@ -742,6 +744,12 @@ public class ArenaImpl implements Arena
             return false;
         }
 
+        // Protect against infinite leave loops
+        if (leavingPlayers.contains(p)) {
+            return false;
+        }
+        leavingPlayers.add(p);
+
         // Clear inventory if player is an arena player, and unmount
         if (arenaPlayers.contains(p)) {
             unmount(p);
@@ -772,9 +780,16 @@ public class ArenaImpl implements Arena
         discardPlayer(p);
         
         endArena();
+
+        leavingPlayers.remove(p);
         return true;
     }
 
+    @Override
+    public boolean isLeaving(Player p) {
+        return leavingPlayers.contains(p);
+    }
+
     @Override
     public void playerDeath(Player p)
     {
diff --git a/src/main/java/com/garbagemule/MobArena/ArenaListener.java b/src/main/java/com/garbagemule/MobArena/ArenaListener.java
index e950ac7..dc2e676 100644
--- a/src/main/java/com/garbagemule/MobArena/ArenaListener.java
+++ b/src/main/java/com/garbagemule/MobArena/ArenaListener.java
@@ -942,6 +942,21 @@ public class ArenaListener
     public void onPlayerDropItem(PlayerDropItemEvent event) {
         Player p = event.getPlayer();
 
+        /*
+         * If the player "drops an item" while in the process of leaving the
+         * arena, it has to be due to something that happens in the leaving
+         * process because most of the event system is single-threaded. This
+         * doesn't make much sense, but it can happen if the player earns a
+         * command reward of /give, which drops the item with a drop delay of
+         * 0 from the player, causing the player to immediately pick it up.
+         * Cancelling this event causes the player to somehow pick up the item
+         * twice, so we don't want to do that. This early return should guard
+         * against this specific case, and hopefully not break anything else.
+         */
+        if (arena.isLeaving(p)) {
+            return;
+        }
+
         // If the player is active in the arena, only cancel if sharing is not allowed
         if (arena.inArena(p)) {
             if (!canShare) {
diff --git a/src/main/java/com/garbagemule/MobArena/framework/Arena.java b/src/main/java/com/garbagemule/MobArena/framework/Arena.java
index 38557db..70e2607 100644
--- a/src/main/java/com/garbagemule/MobArena/framework/Arena.java
+++ b/src/main/java/com/garbagemule/MobArena/framework/Arena.java
@@ -153,6 +153,8 @@ public interface Arena
     void playerReady(Player p);
     
     boolean playerLeave(Player p);
+
+    boolean isLeaving(Player p);
     
     void playerDeath(Player p);