Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions src/main/java/studio/magemonkey/fabled/api/util/FlagManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<Integer, FlagData> data = new HashMap<>();
private static final Map<Integer, FlagData> 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<Integer> clearingEntities = new HashSet<>();

/**
* Retrieves the flag data for an entity. This creates new data if
Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
@@ -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));
}
}
Loading