From 63991bc68496ffd60345b1c135f9a23ca36d1ebf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 18:15:58 +0000 Subject: [PATCH 1/2] fix: prevent infinite recursion in FlagManager.clearFlags Split out of #1677 (piece 4 of 8). If a FlagExpireTrigger-driven skill adds a new flag to an entity while FlagManager.clearFlags() is clearing that same entity's flags, the new flag re-creates a FlagData entry, and removing it during the same clear pass re-enters clearFlags() for that entity, recursing indefinitely (observed on player death, where clearing flags can trigger skills that re-flag the dying entity). Guard clearFlags() with a per-entity "currently clearing" set so a reentrant call for the same entity is a no-op instead of recursing. --- .../fabled/api/util/FlagManager.java | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/main/java/studio/magemonkey/fabled/api/util/FlagManager.java b/src/main/java/studio/magemonkey/fabled/api/util/FlagManager.java index e4596380a8..debeba9c37 100644 --- a/src/main/java/studio/magemonkey/fabled/api/util/FlagManager.java +++ b/src/main/java/studio/magemonkey/fabled/api/util/FlagManager.java @@ -29,13 +29,22 @@ import org.bukkit.entity.LivingEntity; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; /** * The manager for temporary entity flag data */ public class FlagManager { - private static final Map data = new HashMap<>(); + private static final Map data = new HashMap<>(); + /** + * Guards against reentrant clearFlags calls on the same entity. + * Prevents infinite recursion when FlagExpireTrigger skills add new flags + * during flag-clearing (e.g., on player death), which would re-create a + * FlagData entry and cause clearFlags → clear → removeFlag → clearFlags → ∞. + */ + private static final Set clearingEntities = new HashSet<>(); /** * Retrieves the flag data for an entity. This creates new data if @@ -128,9 +137,21 @@ public static void clearFlags(LivingEntity entity) { if (entity == null) { return; } - FlagData result = data.remove(entity.getEntityId()); - if (result != null) { - result.clear(); + // Guard against reentrant calls on the same entity. + // FlagExpireTrigger skills may call addFlag during clearing, + // re-creating a FlagData entry; without this guard that would + // cause infinite recursion (clearFlags → clear → removeFlag → clearFlags …). + int id = entity.getEntityId(); + if (!clearingEntities.add(id)) { + return; // already being cleared — skip + } + try { + FlagData result = data.remove(id); + if (result != null) { + result.clear(); + } + } finally { + clearingEntities.remove(id); } } } From 716d01689ae138ceb0cfc6abf9fd13adb3e1abfa Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 18:25:04 +0000 Subject: [PATCH 2/2] test: cover FlagManager.clearFlags reentrancy guard Reproduces the infinite-recursion scenario the guard fixes: a FlagExpireTrigger-driven skill reacting to FlagExpireEvent by adding a new flag to the same entity while clearFlags() is still unwinding for that entity. Without the clearingEntities guard, this recurses through clear() -> removeFlag() -> (synchronous event) -> addFlag() -> clearFlags() until the stack overflows; with it, the reentrant call is a no-op and the newly-added flag survives. --- .../api/util/FlagManagerReentrancyTest.java | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 src/test/java/studio/magemonkey/fabled/api/util/FlagManagerReentrancyTest.java diff --git a/src/test/java/studio/magemonkey/fabled/api/util/FlagManagerReentrancyTest.java b/src/test/java/studio/magemonkey/fabled/api/util/FlagManagerReentrancyTest.java new file mode 100644 index 0000000000..fcc50bc605 --- /dev/null +++ b/src/test/java/studio/magemonkey/fabled/api/util/FlagManagerReentrancyTest.java @@ -0,0 +1,87 @@ +package studio.magemonkey.fabled.api.util; + +import org.bukkit.entity.LivingEntity; +import org.bukkit.event.EventPriority; +import org.bukkit.event.Listener; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import studio.magemonkey.fabled.api.event.FlagExpireEvent; +import studio.magemonkey.fabled.testutil.MockedTest; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Covers the infinite-recursion fix in FlagManager.clearFlags: a FlagExpireTrigger-driven + * skill can add a new flag to an entity while that entity's flags are being cleared (e.g. + * on death), which previously re-entered clearFlags for the same entity and recursed forever. + */ +public class FlagManagerReentrancyTest extends MockedTest implements Listener { + private LivingEntity entity; + + @BeforeEach + void setup() { + entity = genPlayer("ReentrancyTester"); + } + + private void reflagOnExpire(String expiredFlag, String newFlag) { + server.getPluginManager().registerEvent(FlagExpireEvent.class, this, EventPriority.NORMAL, + (listener, event) -> { + FlagExpireEvent e = (FlagExpireEvent) event; + if (e.getEntity().equals(entity) && e.getFlag().equals(expiredFlag)) { + FlagManager.addFlag(entity, newFlag, 100); + } + }, plugin, true); + } + + @Test + void clearFlags_reentrantAddDuringExpire_doesNotRecurseForever() { + reflagOnExpire("initial", "reflag"); + + FlagManager.addFlag(entity, "initial", 100); + + assertDoesNotThrow(() -> FlagManager.clearFlags(entity)); + } + + @Test + void clearFlags_reentrantAdd_leavesNewlyAddedFlagIntact() { + reflagOnExpire("initial", "reflag"); + + FlagManager.addFlag(entity, "initial", 100); + FlagManager.clearFlags(entity); + + assertFalse(FlagManager.hasFlag(entity, "initial")); + assertTrue(FlagManager.hasFlag(entity, "reflag")); + } + + @Test + void clearFlags_selfReflagLoop_doesNotRecurseForever() { + // A skill that keeps re-adding the same flag on every expire would, without the + // guard, cause clear() -> removeFlag() -> expire event -> addFlag() -> (flags now + // empty again) -> clearFlags() -> clear() -> ... indefinitely for the same entity. + reflagOnExpire("loopy", "loopy"); + + FlagManager.addFlag(entity, "loopy", 100); + + assertDoesNotThrow(() -> FlagManager.clearFlags(entity)); + // The flag re-added mid-clear should survive the clear rather than being dropped. + assertTrue(FlagManager.hasFlag(entity, "loopy")); + } + + @Test + void clearFlags_normalCase_clearsAllFlags() { + FlagManager.addFlag(entity, "a", 100); + FlagManager.addFlag(entity, "b", 100); + + FlagManager.clearFlags(entity); + + assertFalse(FlagManager.hasFlag(entity, "a")); + assertFalse(FlagManager.hasFlag(entity, "b")); + } + + @Test + void clearFlags_nullEntity_doesNotThrow() { + assertDoesNotThrow(() -> FlagManager.clearFlags(null)); + } +}