From 4133360d10188dd71f5dc604430b8d9849640305 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 19:17:13 -0600 Subject: [PATCH 01/19] Fix full inventory lifecycle and thread safety --- .../api/item/FullInventoryHandler.java | 274 +++++++++++------- 1 file changed, 168 insertions(+), 106 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index 029cdd8423..716ad9c5ab 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -1,16 +1,19 @@ package com.bencodez.advancedcore.api.item; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; +import java.util.logging.Level; import org.bukkit.Bukkit; +import org.bukkit.configuration.ConfigurationSection; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; @@ -23,31 +26,36 @@ * Handler for items when player inventories are full. */ public class FullInventoryHandler { + private static final long MESSAGE_COOLDOWN_MS = 5000L; + private static final long PENDING_ITEM_RETENTION_MS = TimeUnit.DAYS.toMillis(1); + /** * The items waiting to be given. - * + * * @return the items waiting to be given */ @Getter - private ConcurrentHashMap> items; + private final ConcurrentHashMap> items = new ConcurrentHashMap<>(); - private AdvancedCorePlugin plugin; + private final AdvancedCorePlugin plugin; /** - * The timer executor service. - * + * The shared inventory timer executor service. + * * @return the timer executor service */ @Getter private ScheduledExecutorService timer; + private ScheduledFuture checkTask; + /** * The last message time for each player. - * + * * @return the last message time for each player */ @Getter - private ConcurrentHashMap lastMessageTime; + private final ConcurrentHashMap lastMessageTime = new ConcurrentHashMap<>(); /** * Constructor for FullInventoryHandler. @@ -55,8 +63,6 @@ public class FullInventoryHandler { * @param plugin the plugin instance */ public FullInventoryHandler(AdvancedCorePlugin plugin) { - items = new ConcurrentHashMap<>(); - lastMessageTime = new ConcurrentHashMap<>(); this.plugin = plugin; loadTimer(); startup(); @@ -69,13 +75,7 @@ public FullInventoryHandler(AdvancedCorePlugin plugin) { * @param item the items to add */ public void add(UUID uuid, ArrayList item) { - if (items.containsKey(uuid)) { - ArrayList current = items.get(uuid); - current.addAll(item); - items.put(null, current); - } else { - items.put(uuid, item); - } + addItems(uuid, item); } /** @@ -85,28 +85,33 @@ public void add(UUID uuid, ArrayList item) { * @param item the item to add */ public void add(UUID uuid, ItemStack item) { - if (items.containsKey(uuid)) { - ArrayList current = items.get(uuid); - current.add(item); - items.put(uuid, current); - } else { - ArrayList itemList = new ArrayList<>(); - itemList.add(item); - items.put(uuid, itemList); + if (item == null) { + return; } + ArrayList itemList = new ArrayList<>(); + itemList.add(item); + addItems(uuid, itemList); } /** - * Checks all players for pending items. + * Checks all players for pending items. Bukkit player/inventory access is always + * moved to the Bukkit scheduler when this method is invoked asynchronously. */ public void check() { - for (UUID entry : items.keySet()) { - Player p = Bukkit.getPlayer(entry); - check(p); - if (lastMessageTime.containsKey(entry)) { - if ((System.currentTimeMillis() - lastMessageTime.get(p.getUniqueId()).longValue()) > 5000) { - lastMessageTime.remove(entry); - } + if (!Bukkit.isPrimaryThread()) { + plugin.getBukkitScheduler().runTask(plugin, this::check); + return; + } + + long now = System.currentTimeMillis(); + for (UUID uuid : new ArrayList<>(items.keySet())) { + Player player = Bukkit.getPlayer(uuid); + if (player != null) { + check(player); + } + Long lastMessage = lastMessageTime.get(uuid); + if (lastMessage != null && now - lastMessage.longValue() > MESSAGE_COOLDOWN_MS) { + lastMessageTime.remove(uuid, lastMessage); } } } @@ -114,70 +119,99 @@ public void check() { /** * Checks a specific player for pending items. * - * @param p the player + * @param player the player */ - public void check(Player p) { - if (p != null && items.containsKey(p.getUniqueId())) { - ArrayList extra = new ArrayList<>(); - for (ItemStack item : items.get(p.getUniqueId())) { - HashMap excess = p.getInventory().addItem(item); - for (Map.Entry me : excess.entrySet()) { - extra.add(me.getValue()); - } - } - if (extra.size() == 0) { - items.remove(p.getUniqueId()); - } else { - items.put(p.getUniqueId(), extra); + public void check(Player player) { + if (player == null) { + return; + } + if (!Bukkit.isPrimaryThread()) { + plugin.getBukkitScheduler().runTask(plugin, () -> check(player), player); + return; + } + + UUID uuid = player.getUniqueId(); + ArrayList pending = items.remove(uuid); + if (pending == null || pending.isEmpty()) { + return; + } + + ArrayList extra = new ArrayList<>(); + for (ItemStack item : pending) { + if (item == null) { + continue; } + HashMap excess = player.getInventory().addItem(item); + extra.addAll(excess.values()); + } + if (!extra.isEmpty()) { + addItems(uuid, extra); } } /** - * Gives items to a player. + * Gives items to a player. Bukkit inventory/world operations are moved to the + * Bukkit scheduler when called asynchronously. * - * @param p the player + * @param player the player * @param item the items to give */ - public void giveItem(Player p, ItemStack... item) { - HashMap excess = p.getInventory().addItem(item); - boolean full = false; - boolean dropItems = plugin.getOptions().isDropOnFullInv(); + public void giveItem(Player player, ItemStack... item) { + if (player == null || item == null || item.length == 0) { + return; + } + if (!Bukkit.isPrimaryThread()) { + ItemStack[] itemsToGive = item.clone(); + plugin.getBukkitScheduler().runTask(plugin, () -> giveItem(player, itemsToGive), player); + return; + } + + HashMap excess = player.getInventory().addItem(item); + if (excess.isEmpty()) { + player.updateInventory(); + return; + } - for (Map.Entry me : excess.entrySet()) { - full = true; + boolean dropItems = plugin.getOptions().isDropOnFullInv(); + for (ItemStack extra : excess.values()) { if (dropItems) { - p.getWorld().dropItem(p.getLocation(), me.getValue()); + player.getWorld().dropItem(player.getLocation(), extra); } else { - add(p.getUniqueId(), me.getValue()); + add(player.getUniqueId(), extra); } } - if (full) { - if (lastMessageTime.containsKey(p.getUniqueId())) { - if ((System.currentTimeMillis() - lastMessageTime.get(p.getUniqueId()).longValue()) > 5000) { - sendMessage(p); - } - } else { - sendMessage(p); - } + if (shouldSendMessage(player.getUniqueId())) { + sendMessage(player); } - - p.updateInventory(); + player.updateInventory(); } /** - * Loads the timer for checking inventories. + * Loads the timer for checking inventories. The handler reuses AdvancedCore's + * inventory executor and schedules Bukkit work back through the Bukkit scheduler. */ - public void loadTimer() { - timer = Executors.newScheduledThreadPool(1); - timer.scheduleAtFixedRate(new Runnable() { + public synchronized void loadTimer() { + if (checkTask != null && !checkTask.isDone() && !checkTask.isCancelled()) { + return; + } + timer = plugin.getInventoryTimer(); + if (timer == null || timer.isShutdown()) { + return; + } + checkTask = timer.scheduleAtFixedRate(() -> plugin.getBukkitScheduler().runTask(plugin, this::check), 10, 30, + TimeUnit.SECONDS); + } - @Override - public void run() { - check(); - } - }, 10, 30, TimeUnit.SECONDS); + /** + * Stops this handler's repeating task without shutting down the shared inventory + * executor. + */ + public synchronized void shutdown() { + if (checkTask != null) { + checkTask.cancel(false); + checkTask = null; + } } /** @@ -185,28 +219,19 @@ public void run() { */ public void save() { try { - if (plugin.getServerDataFile().getData() == null) { + if (plugin.getServerDataFile() == null || plugin.getServerDataFile().getData() == null) { return; } + plugin.getServerDataFile().setData("FullInventory", null); for (Entry> entry : items.entrySet()) { - ArrayList items = entry.getValue(); - for (int i = 0; i < items.size(); i++) { - plugin.getServerDataFile().setData("FullInventory." + entry.getKey().toString() + ".Items." + i, - items.get(i)); + ArrayList pending = new ArrayList<>(entry.getValue()); + for (int i = 0; i < pending.size(); i++) { + plugin.getServerDataFile().setData("FullInventory." + entry.getKey() + ".Items." + i, pending.get(i)); } - plugin.getServerDataFile().setData("FullInventory." + entry.getKey().toString() + ".Time", - System.currentTimeMillis()); + plugin.getServerDataFile().setData("FullInventory." + entry.getKey() + ".Time", System.currentTimeMillis()); } } catch (Exception e) { - e.printStackTrace(); - } - } - - private void sendMessage(Player p) { - String msg = MessageAPI.colorize(AdvancedCorePlugin.getInstance().getOptions().getFormatInvFull()); - if (!msg.isEmpty()) { - p.sendMessage(msg); - lastMessageTime.put(p.getUniqueId(), System.currentTimeMillis()); + plugin.getLogger().log(Level.WARNING, "Failed to save pending full-inventory items", e); } } @@ -215,31 +240,68 @@ private void sendMessage(Player p) { */ public void startup() { try { - if (plugin.getServerDataFile().getData() == null - || !plugin.getServerDataFile().getData().isConfigurationSection("FullInventory")) { + if (plugin.getServerDataFile() == null || plugin.getServerDataFile().getData() == null) { + return; + } + ConfigurationSection root = plugin.getServerDataFile().getData().getConfigurationSection("FullInventory"); + if (root == null) { return; } - for (String uuid : plugin.getServerDataFile().getData().getConfigurationSection("FullInventory") - .getKeys(false)) { - - // check time to keep a lot of items from long time offline players - long time = plugin.getServerDataFile().getData().getLong("FullInventory." + uuid + ".Time"); - if (System.currentTimeMillis() - time < (1000 * 60 * 60 * 24)) { - - for (String itemnum : plugin.getServerDataFile().getData() - .getConfigurationSection("FullInventory." + uuid + ".Items").getKeys(false)) { - add(UUID.fromString(uuid), plugin.getServerDataFile().getData() - .getItemStack("FullInventory." + uuid + ".Items." + itemnum)); + long now = System.currentTimeMillis(); + for (String uuidString : root.getKeys(false)) { + try { + UUID uuid = UUID.fromString(uuidString); + long time = root.getLong(uuidString + ".Time"); + if (now - time >= PENDING_ITEM_RETENTION_MS) { + continue; + } + ConfigurationSection itemSection = root.getConfigurationSection(uuidString + ".Items"); + if (itemSection == null) { + continue; } + for (String itemNumber : itemSection.getKeys(false)) { + ItemStack item = itemSection.getItemStack(itemNumber); + if (item != null) { + add(uuid, item); + } + } + } catch (IllegalArgumentException e) { + plugin.getLogger().warning("Skipping invalid FullInventory UUID entry: " + uuidString); } } plugin.getServerDataFile().setData("FullInventory", null); } catch (Exception e) { - e.printStackTrace(); + plugin.getLogger().log(Level.WARNING, "Failed to load pending full-inventory items", e); + } + } + + private void addItems(UUID uuid, Collection itemsToAdd) { + if (uuid == null || itemsToAdd == null || itemsToAdd.isEmpty()) { + return; } + items.compute(uuid, (key, current) -> { + ArrayList merged = current == null ? new ArrayList<>() : new ArrayList<>(current); + for (ItemStack item : itemsToAdd) { + if (item != null) { + merged.add(item); + } + } + return merged.isEmpty() ? null : merged; + }); + } + private boolean shouldSendMessage(UUID uuid) { + Long lastMessage = lastMessageTime.get(uuid); + return lastMessage == null || System.currentTimeMillis() - lastMessage.longValue() > MESSAGE_COOLDOWN_MS; } + private void sendMessage(Player player) { + String msg = MessageAPI.colorize(plugin.getOptions().getFormatInvFull()); + if (!msg.isEmpty()) { + player.sendMessage(msg); + lastMessageTime.put(player.getUniqueId(), System.currentTimeMillis()); + } + } } From b281ae1c57cc30ae5b12a07ff66b8aea77566cf1 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 19:17:44 -0600 Subject: [PATCH 02/19] Test full inventory correctness and scheduling --- .../tests/item/FullInventoryHandlerTest.java | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java new file mode 100644 index 0000000000..086378a0a2 --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java @@ -0,0 +1,147 @@ +package com.bencodez.advancedcore.tests.item; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.UUID; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +import org.bukkit.Bukkit; +import org.bukkit.configuration.file.YamlConfiguration; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; + +import com.bencodez.advancedcore.AdvancedCorePlugin; +import com.bencodez.advancedcore.api.item.FullInventoryHandler; +import com.bencodez.advancedcore.data.ServerData; +import com.bencodez.simpleapi.scheduler.BukkitScheduler; + +public class FullInventoryHandlerTest { + + @Test + public void addListMergesUnderSameUuid() { + Fixture fixture = createFixture(); + UUID uuid = UUID.randomUUID(); + ItemStack first = mock(ItemStack.class); + ItemStack second = mock(ItemStack.class); + + fixture.handler.add(uuid, new ArrayList<>(java.util.List.of(first))); + fixture.handler.add(uuid, new ArrayList<>(java.util.List.of(second))); + + assertEquals(java.util.List.of(first, second), fixture.handler.getItems().get(uuid)); + assertFalse(fixture.handler.getItems().containsKey(null)); + } + + @Test + public void offlinePlayerDoesNotBreakPendingSweep() { + Fixture fixture = createFixture(); + UUID uuid = UUID.randomUUID(); + fixture.handler.add(uuid, mock(ItemStack.class)); + fixture.handler.getLastMessageTime().put(uuid, 0L); + + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::isPrimaryThread).thenReturn(true); + bukkit.when(() -> Bukkit.getPlayer(uuid)).thenReturn(null); + + assertDoesNotThrow(fixture.handler::check); + } + + assertFalse(fixture.handler.getLastMessageTime().containsKey(uuid)); + } + + @Test + public void timerOnlySchedulesBukkitWork() { + Fixture fixture = createFixture(); + ArgumentCaptor timerTask = ArgumentCaptor.forClass(Runnable.class); + verify(fixture.timer).scheduleAtFixedRate(timerTask.capture(), eq(10L), eq(30L), eq(TimeUnit.SECONDS)); + + timerTask.getValue().run(); + + verify(fixture.bukkitScheduler).runTask(eq(fixture.plugin), any(Runnable.class)); + } + + @Test + public void asyncPlayerCheckMovesToBukkitScheduler() { + Fixture fixture = createFixture(); + Player player = mock(Player.class); + + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::isPrimaryThread).thenReturn(false); + fixture.handler.check(player); + } + + verify(fixture.bukkitScheduler).runTask(eq(fixture.plugin), any(Runnable.class), eq(player)); + } + + @Test + public void asyncGiveItemMovesToBukkitScheduler() { + Fixture fixture = createFixture(); + Player player = mock(Player.class); + ItemStack item = mock(ItemStack.class); + + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::isPrimaryThread).thenReturn(false); + fixture.handler.giveItem(player, item); + } + + verify(fixture.bukkitScheduler).runTask(eq(fixture.plugin), any(Runnable.class), eq(player)); + } + + @Test + public void shutdownCancelsOnlyHandlerTask() { + Fixture fixture = createFixture(); + + fixture.handler.shutdown(); + + verify(fixture.future).cancel(false); + } + + private Fixture createFixture() { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); + ScheduledExecutorService timer = mock(ScheduledExecutorService.class); + ScheduledFuture future = mock(ScheduledFuture.class); + BukkitScheduler bukkitScheduler = mock(BukkitScheduler.class); + ServerData serverData = mock(ServerData.class); + YamlConfiguration data = new YamlConfiguration(); + + when(plugin.getInventoryTimer()).thenReturn(timer); + when(plugin.getBukkitScheduler()).thenReturn(bukkitScheduler); + when(plugin.getServerDataFile()).thenReturn(serverData); + when(serverData.getData()).thenReturn(data); + doReturn(future).when(timer).scheduleAtFixedRate(any(Runnable.class), eq(10L), eq(30L), eq(TimeUnit.SECONDS)); + + FullInventoryHandler handler = new FullInventoryHandler(plugin); + return new Fixture(plugin, timer, future, bukkitScheduler, handler); + } + + private static final class Fixture { + private final AdvancedCorePlugin plugin; + private final ScheduledExecutorService timer; + private final ScheduledFuture future; + private final BukkitScheduler bukkitScheduler; + private final FullInventoryHandler handler; + + private Fixture(AdvancedCorePlugin plugin, ScheduledExecutorService timer, ScheduledFuture future, + BukkitScheduler bukkitScheduler, FullInventoryHandler handler) { + this.plugin = plugin; + this.timer = timer; + this.future = future; + this.bukkitScheduler = bukkitScheduler; + this.handler = handler; + } + } +} From e8bf02eef29bbfcd0e2d258793cd48b13ec5c4ab Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 19:21:56 -0600 Subject: [PATCH 03/19] Fix full inventory test compilation --- .../advancedcore/tests/item/FullInventoryHandlerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java index 086378a0a2..dccb9035f0 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java @@ -57,7 +57,7 @@ public void offlinePlayerDoesNotBreakPendingSweep() { bukkit.when(Bukkit::isPrimaryThread).thenReturn(true); bukkit.when(() -> Bukkit.getPlayer(uuid)).thenReturn(null); - assertDoesNotThrow(fixture.handler::check); + assertDoesNotThrow(() -> fixture.handler.check()); } assertFalse(fixture.handler.getLastMessageTime().containsKey(uuid)); From b3a823869ce3973ab1865452b3e0212fc9e87cee Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 19:23:43 -0600 Subject: [PATCH 04/19] Fix null-key regression assertion --- .../advancedcore/tests/item/FullInventoryHandlerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java index dccb9035f0..0f12678e7c 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java @@ -43,7 +43,7 @@ public void addListMergesUnderSameUuid() { fixture.handler.add(uuid, new ArrayList<>(java.util.List.of(second))); assertEquals(java.util.List.of(first, second), fixture.handler.getItems().get(uuid)); - assertFalse(fixture.handler.getItems().containsKey(null)); + assertEquals(1, fixture.handler.getItems().size()); } @Test From 335be4b1f948aa321ca1b2b13b898c591842034e Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 19:43:56 -0600 Subject: [PATCH 05/19] Schedule full inventory sweeps per player region --- .../api/item/FullInventoryHandler.java | 78 ++++++++++--------- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index 716ad9c5ab..9492c320c7 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -94,26 +94,16 @@ public void add(UUID uuid, ItemStack item) { } /** - * Checks all players for pending items. Bukkit player/inventory access is always - * moved to the Bukkit scheduler when this method is invoked asynchronously. + * Checks all players for pending items. The sweep itself runs on the normal + * Bukkit/global scheduler, but every player inventory operation is scheduled on + * that player's scheduler/region before it is executed. */ public void check() { if (!Bukkit.isPrimaryThread()) { - plugin.getBukkitScheduler().runTask(plugin, this::check); + plugin.getBukkitScheduler().runTask(plugin, this::schedulePendingPlayerChecks); return; } - - long now = System.currentTimeMillis(); - for (UUID uuid : new ArrayList<>(items.keySet())) { - Player player = Bukkit.getPlayer(uuid); - if (player != null) { - check(player); - } - Long lastMessage = lastMessageTime.get(uuid); - if (lastMessage != null && now - lastMessage.longValue() > MESSAGE_COOLDOWN_MS) { - lastMessageTime.remove(uuid, lastMessage); - } - } + schedulePendingPlayerChecks(); } /** @@ -126,27 +116,10 @@ public void check(Player player) { return; } if (!Bukkit.isPrimaryThread()) { - plugin.getBukkitScheduler().runTask(plugin, () -> check(player), player); - return; - } - - UUID uuid = player.getUniqueId(); - ArrayList pending = items.remove(uuid); - if (pending == null || pending.isEmpty()) { + plugin.getBukkitScheduler().runTask(plugin, () -> checkOwnedPlayer(player), player); return; } - - ArrayList extra = new ArrayList<>(); - for (ItemStack item : pending) { - if (item == null) { - continue; - } - HashMap excess = player.getInventory().addItem(item); - extra.addAll(excess.values()); - } - if (!extra.isEmpty()) { - addItems(uuid, extra); - } + checkOwnedPlayer(player); } /** @@ -199,7 +172,8 @@ public synchronized void loadTimer() { if (timer == null || timer.isShutdown()) { return; } - checkTask = timer.scheduleAtFixedRate(() -> plugin.getBukkitScheduler().runTask(plugin, this::check), 10, 30, + checkTask = timer.scheduleAtFixedRate( + () -> plugin.getBukkitScheduler().runTask(plugin, this::schedulePendingPlayerChecks), 10, 30, TimeUnit.SECONDS); } @@ -292,6 +266,40 @@ private void addItems(UUID uuid, Collection itemsToAdd) { }); } + private void checkOwnedPlayer(Player player) { + UUID uuid = player.getUniqueId(); + ArrayList pending = items.remove(uuid); + if (pending == null || pending.isEmpty()) { + return; + } + + ArrayList extra = new ArrayList<>(); + for (ItemStack item : pending) { + if (item == null) { + continue; + } + HashMap excess = player.getInventory().addItem(item); + extra.addAll(excess.values()); + } + if (!extra.isEmpty()) { + addItems(uuid, extra); + } + } + + private void schedulePendingPlayerChecks() { + long now = System.currentTimeMillis(); + for (UUID uuid : new ArrayList<>(items.keySet())) { + Player player = Bukkit.getPlayer(uuid); + if (player != null) { + plugin.getBukkitScheduler().runTask(plugin, () -> checkOwnedPlayer(player), player); + } + Long lastMessage = lastMessageTime.get(uuid); + if (lastMessage != null && now - lastMessage.longValue() > MESSAGE_COOLDOWN_MS) { + lastMessageTime.remove(uuid, lastMessage); + } + } + } + private boolean shouldSendMessage(UUID uuid) { Long lastMessage = lastMessageTime.get(uuid); return lastMessage == null || System.currentTimeMillis() - lastMessage.longValue() > MESSAGE_COOLDOWN_MS; From 1254a1d85aa992bb4b7075352321826ac7d4b569 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 19:44:17 -0600 Subject: [PATCH 06/19] Test per-player full inventory sweep scheduling --- .../tests/item/FullInventoryHandlerTest.java | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java index 0f12678e7c..f2223e799c 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java @@ -64,7 +64,7 @@ public void offlinePlayerDoesNotBreakPendingSweep() { } @Test - public void timerOnlySchedulesBukkitWork() { + public void timerSchedulesGlobalDiscoveryOnly() { Fixture fixture = createFixture(); ArgumentCaptor timerTask = ArgumentCaptor.forClass(Runnable.class); verify(fixture.timer).scheduleAtFixedRate(timerTask.capture(), eq(10L), eq(30L), eq(TimeUnit.SECONDS)); @@ -74,6 +74,23 @@ public void timerOnlySchedulesBukkitWork() { verify(fixture.bukkitScheduler).runTask(eq(fixture.plugin), any(Runnable.class)); } + @Test + public void pendingSweepSchedulesEachOnlinePlayerOnEntityScheduler() { + Fixture fixture = createFixture(); + UUID uuid = UUID.randomUUID(); + Player player = mock(Player.class); + fixture.handler.add(uuid, mock(ItemStack.class)); + + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::isPrimaryThread).thenReturn(true); + bukkit.when(() -> Bukkit.getPlayer(uuid)).thenReturn(player); + + fixture.handler.check(); + } + + verify(fixture.bukkitScheduler).runTask(eq(fixture.plugin), any(Runnable.class), eq(player)); + } + @Test public void asyncPlayerCheckMovesToBukkitScheduler() { Fixture fixture = createFixture(); From 360ba12b7faf65ded5c7adba7337806fa8f0c367 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 19:51:25 -0600 Subject: [PATCH 07/19] Always schedule player inventory work on entity region --- .../api/item/FullInventoryHandler.java | 65 +++++++++---------- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index 9492c320c7..16f5dca50f 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -3,7 +3,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.Map; import java.util.Map.Entry; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -107,7 +106,9 @@ public void check() { } /** - * Checks a specific player for pending items. + * Checks a specific player for pending items. The inventory work is always + * scheduled through that player's entity scheduler so Folia region ownership is + * respected even when the caller is already on a different tick/region thread. * * @param player the player */ @@ -115,16 +116,13 @@ public void check(Player player) { if (player == null) { return; } - if (!Bukkit.isPrimaryThread()) { - plugin.getBukkitScheduler().runTask(plugin, () -> checkOwnedPlayer(player), player); - return; - } - checkOwnedPlayer(player); + plugin.getBukkitScheduler().runTask(plugin, () -> checkOwnedPlayer(player), player); } /** - * Gives items to a player. Bukkit inventory/world operations are moved to the - * Bukkit scheduler when called asynchronously. + * Gives items to a player. Inventory/world operations are always scheduled + * through that player's entity scheduler so the public API is safe when called + * from async code, the global region, or another entity's region. * * @param player the player * @param item the items to give @@ -133,31 +131,8 @@ public void giveItem(Player player, ItemStack... item) { if (player == null || item == null || item.length == 0) { return; } - if (!Bukkit.isPrimaryThread()) { - ItemStack[] itemsToGive = item.clone(); - plugin.getBukkitScheduler().runTask(plugin, () -> giveItem(player, itemsToGive), player); - return; - } - - HashMap excess = player.getInventory().addItem(item); - if (excess.isEmpty()) { - player.updateInventory(); - return; - } - - boolean dropItems = plugin.getOptions().isDropOnFullInv(); - for (ItemStack extra : excess.values()) { - if (dropItems) { - player.getWorld().dropItem(player.getLocation(), extra); - } else { - add(player.getUniqueId(), extra); - } - } - - if (shouldSendMessage(player.getUniqueId())) { - sendMessage(player); - } - player.updateInventory(); + ItemStack[] itemsToGive = item.clone(); + plugin.getBukkitScheduler().runTask(plugin, () -> giveItemOwnedPlayer(player, itemsToGive), player); } /** @@ -286,6 +261,28 @@ private void checkOwnedPlayer(Player player) { } } + private void giveItemOwnedPlayer(Player player, ItemStack[] item) { + HashMap excess = player.getInventory().addItem(item); + if (excess.isEmpty()) { + player.updateInventory(); + return; + } + + boolean dropItems = plugin.getOptions().isDropOnFullInv(); + for (ItemStack extra : excess.values()) { + if (dropItems) { + player.getWorld().dropItem(player.getLocation(), extra); + } else { + add(player.getUniqueId(), extra); + } + } + + if (shouldSendMessage(player.getUniqueId())) { + sendMessage(player); + } + player.updateInventory(); + } + private void schedulePendingPlayerChecks() { long now = System.currentTimeMillis(); for (UUID uuid : new ArrayList<>(items.keySet())) { From ce8c383ab882739b31df847a11bd1adf346d3214 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 19:51:45 -0600 Subject: [PATCH 08/19] Test Folia-safe public inventory entry points --- .../tests/item/FullInventoryHandlerTest.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java index f2223e799c..8ae89dccd3 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java @@ -92,28 +92,22 @@ public void pendingSweepSchedulesEachOnlinePlayerOnEntityScheduler() { } @Test - public void asyncPlayerCheckMovesToBukkitScheduler() { + public void playerCheckAlwaysUsesEntityScheduler() { Fixture fixture = createFixture(); Player player = mock(Player.class); - try (MockedStatic bukkit = mockStatic(Bukkit.class)) { - bukkit.when(Bukkit::isPrimaryThread).thenReturn(false); - fixture.handler.check(player); - } + fixture.handler.check(player); verify(fixture.bukkitScheduler).runTask(eq(fixture.plugin), any(Runnable.class), eq(player)); } @Test - public void asyncGiveItemMovesToBukkitScheduler() { + public void giveItemAlwaysUsesEntityScheduler() { Fixture fixture = createFixture(); Player player = mock(Player.class); ItemStack item = mock(ItemStack.class); - try (MockedStatic bukkit = mockStatic(Bukkit.class)) { - bukkit.when(Bukkit::isPrimaryThread).thenReturn(false); - fixture.handler.giveItem(player, item); - } + fixture.handler.giveItem(player, item); verify(fixture.bukkitScheduler).runTask(eq(fixture.plugin), any(Runnable.class), eq(player)); } From 740f6c843ed4adba7b246ee3f141610bf3051700 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:04:27 -0600 Subject: [PATCH 09/19] Preserve full inventory timer isolation --- .../api/item/FullInventoryHandler.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index 16f5dca50f..d43fca756e 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -6,6 +6,7 @@ import java.util.Map.Entry; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -39,7 +40,9 @@ public class FullInventoryHandler { private final AdvancedCorePlugin plugin; /** - * The shared inventory timer executor service. + * The handler-owned timer executor service. This remains isolated from + * AdvancedCore's shared inventory timer so legacy callers of getTimer().shutdown() + * cannot stop unrelated GUI scheduling. * * @return the timer executor service */ @@ -136,16 +139,16 @@ public void giveItem(Player player, ItemStack... item) { } /** - * Loads the timer for checking inventories. The handler reuses AdvancedCore's - * inventory executor and schedules Bukkit work back through the Bukkit scheduler. + * Loads the timer for checking inventories. The handler keeps its historically + * isolated executor, while actual Bukkit work is handed back to the Bukkit/Folia + * scheduler. */ public synchronized void loadTimer() { if (checkTask != null && !checkTask.isDone() && !checkTask.isCancelled()) { return; } - timer = plugin.getInventoryTimer(); - if (timer == null || timer.isShutdown()) { - return; + if (timer == null || timer.isShutdown() || timer.isTerminated()) { + timer = Executors.newSingleThreadScheduledExecutor(); } checkTask = timer.scheduleAtFixedRate( () -> plugin.getBukkitScheduler().runTask(plugin, this::schedulePendingPlayerChecks), 10, 30, @@ -153,14 +156,16 @@ public synchronized void loadTimer() { } /** - * Stops this handler's repeating task without shutting down the shared inventory - * executor. + * Stops this handler's repeating task and its handler-owned executor. */ public synchronized void shutdown() { if (checkTask != null) { checkTask.cancel(false); checkTask = null; } + if (timer != null) { + timer.shutdownNow(); + } } /** From 37f0a43783d4d67394249dd24d0de33ee55aae8f Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:04:52 -0600 Subject: [PATCH 10/19] Test handler timer isolation --- .../tests/item/FullInventoryHandlerTest.java | 69 +++++++++++-------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java index 8ae89dccd3..f08419686d 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java @@ -3,26 +3,27 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.ArrayList; +import java.util.List; import java.util.UUID; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; import org.bukkit.Bukkit; import org.bukkit.configuration.file.YamlConfiguration; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; import org.mockito.MockedStatic; import com.bencodez.advancedcore.AdvancedCorePlugin; @@ -31,6 +32,14 @@ import com.bencodez.simpleapi.scheduler.BukkitScheduler; public class FullInventoryHandlerTest { + private final List handlers = new ArrayList<>(); + + @AfterEach + public void shutdownHandlers() { + for (FullInventoryHandler handler : handlers) { + handler.shutdown(); + } + } @Test public void addListMergesUnderSameUuid() { @@ -63,17 +72,6 @@ public void offlinePlayerDoesNotBreakPendingSweep() { assertFalse(fixture.handler.getLastMessageTime().containsKey(uuid)); } - @Test - public void timerSchedulesGlobalDiscoveryOnly() { - Fixture fixture = createFixture(); - ArgumentCaptor timerTask = ArgumentCaptor.forClass(Runnable.class); - verify(fixture.timer).scheduleAtFixedRate(timerTask.capture(), eq(10L), eq(30L), eq(TimeUnit.SECONDS)); - - timerTask.getValue().run(); - - verify(fixture.bukkitScheduler).runTask(eq(fixture.plugin), any(Runnable.class)); - } - @Test public void pendingSweepSchedulesEachOnlinePlayerOnEntityScheduler() { Fixture fixture = createFixture(); @@ -113,44 +111,61 @@ public void giveItemAlwaysUsesEntityScheduler() { } @Test - public void shutdownCancelsOnlyHandlerTask() { + public void handlerTimerIsIsolatedFromPluginInventoryTimer() { + Fixture fixture = createFixture(); + ScheduledExecutorService handlerTimer = fixture.handler.getTimer(); + + assertNotSame(fixture.sharedInventoryTimer, handlerTimer); + assertFalse(handlerTimer.isShutdown()); + + handlerTimer.shutdownNow(); + fixture.handler.loadTimer(); + + assertNotSame(handlerTimer, fixture.handler.getTimer()); + assertFalse(fixture.handler.getTimer().isShutdown()); + verify(fixture.sharedInventoryTimer, never()).shutdown(); + verify(fixture.sharedInventoryTimer, never()).shutdownNow(); + } + + @Test + public void shutdownStopsOnlyHandlerTimer() { Fixture fixture = createFixture(); + ScheduledExecutorService handlerTimer = fixture.handler.getTimer(); fixture.handler.shutdown(); - verify(fixture.future).cancel(false); + assertTrue(handlerTimer.isShutdown()); + verify(fixture.sharedInventoryTimer, never()).shutdown(); + verify(fixture.sharedInventoryTimer, never()).shutdownNow(); } private Fixture createFixture() { AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); - ScheduledExecutorService timer = mock(ScheduledExecutorService.class); - ScheduledFuture future = mock(ScheduledFuture.class); + ScheduledExecutorService sharedInventoryTimer = mock(ScheduledExecutorService.class); BukkitScheduler bukkitScheduler = mock(BukkitScheduler.class); ServerData serverData = mock(ServerData.class); YamlConfiguration data = new YamlConfiguration(); - when(plugin.getInventoryTimer()).thenReturn(timer); + when(plugin.getInventoryTimer()).thenReturn(sharedInventoryTimer); when(plugin.getBukkitScheduler()).thenReturn(bukkitScheduler); when(plugin.getServerDataFile()).thenReturn(serverData); when(serverData.getData()).thenReturn(data); - doReturn(future).when(timer).scheduleAtFixedRate(any(Runnable.class), eq(10L), eq(30L), eq(TimeUnit.SECONDS)); FullInventoryHandler handler = new FullInventoryHandler(plugin); - return new Fixture(plugin, timer, future, bukkitScheduler, handler); + handlers.add(handler); + return new Fixture(plugin, sharedInventoryTimer, bukkitScheduler, handler); } private static final class Fixture { private final AdvancedCorePlugin plugin; - private final ScheduledExecutorService timer; - private final ScheduledFuture future; + private final ScheduledExecutorService sharedInventoryTimer; private final BukkitScheduler bukkitScheduler; private final FullInventoryHandler handler; - private Fixture(AdvancedCorePlugin plugin, ScheduledExecutorService timer, ScheduledFuture future, + private Fixture(AdvancedCorePlugin plugin, ScheduledExecutorService sharedInventoryTimer, BukkitScheduler bukkitScheduler, FullInventoryHandler handler) { this.plugin = plugin; - this.timer = timer; - this.future = future; + this.sharedInventoryTimer = sharedInventoryTimer; this.bukkitScheduler = bukkitScheduler; this.handler = handler; } From 162518b8bace1182eaa27ebff5631b7ccf281332 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:06:48 -0600 Subject: [PATCH 11/19] Recover isolated full inventory timer after shutdown --- .../advancedcore/api/item/FullInventoryHandler.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index d43fca756e..b8b5b97b07 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -144,12 +144,16 @@ public void giveItem(Player player, ItemStack... item) { * scheduler. */ public synchronized void loadTimer() { - if (checkTask != null && !checkTask.isDone() && !checkTask.isCancelled()) { - return; - } if (timer == null || timer.isShutdown() || timer.isTerminated()) { + if (checkTask != null) { + checkTask.cancel(false); + checkTask = null; + } timer = Executors.newSingleThreadScheduledExecutor(); } + if (checkTask != null && !checkTask.isDone() && !checkTask.isCancelled()) { + return; + } checkTask = timer.scheduleAtFixedRate( () -> plugin.getBukkitScheduler().runTask(plugin, this::schedulePendingPlayerChecks), 10, 30, TimeUnit.SECONDS); From 786ec78e4a211299b6ee9c244964cd0f8e54d610 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:15:45 -0600 Subject: [PATCH 12/19] Persist full inventory snapshot in one save --- .../api/item/FullInventoryHandler.java | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index b8b5b97b07..ca6e627bb1 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -14,10 +14,13 @@ import org.bukkit.Bukkit; import org.bukkit.configuration.ConfigurationSection; +import org.bukkit.configuration.file.FileConfiguration; +import org.bukkit.configuration.file.YamlConfiguration; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; import com.bencodez.advancedcore.AdvancedCorePlugin; +import com.bencodez.advancedcore.data.ServerData; import com.bencodez.simpleapi.messages.MessageAPI; import lombok.Getter; @@ -40,9 +43,7 @@ public class FullInventoryHandler { private final AdvancedCorePlugin plugin; /** - * The handler-owned timer executor service. This remains isolated from - * AdvancedCore's shared inventory timer so legacy callers of getTimer().shutdown() - * cannot stop unrelated GUI scheduling. + * The handler-owned timer executor service. * * @return the timer executor service */ @@ -145,11 +146,8 @@ public void giveItem(Player player, ItemStack... item) { */ public synchronized void loadTimer() { if (timer == null || timer.isShutdown() || timer.isTerminated()) { - if (checkTask != null) { - checkTask.cancel(false); - checkTask = null; - } timer = Executors.newSingleThreadScheduledExecutor(); + checkTask = null; } if (checkTask != null && !checkTask.isDone() && !checkTask.isCancelled()) { return; @@ -173,21 +171,40 @@ public synchronized void shutdown() { } /** - * Saves pending items to disk. + * Saves pending items to disk. The replacement FullInventory section is built in + * a temporary configuration first, then copied into the live configuration and + * persisted in a single save so an incomplete replacement is never deliberately + * written over the previous recovery snapshot. */ public void save() { try { - if (plugin.getServerDataFile() == null || plugin.getServerDataFile().getData() == null) { + ServerData serverData = plugin.getServerDataFile(); + if (serverData == null || serverData.getData() == null) { return; } - plugin.getServerDataFile().setData("FullInventory", null); + + YamlConfiguration snapshot = new YamlConfiguration(); + long now = System.currentTimeMillis(); for (Entry> entry : items.entrySet()) { ArrayList pending = new ArrayList<>(entry.getValue()); + String basePath = "FullInventory." + entry.getKey(); for (int i = 0; i < pending.size(); i++) { - plugin.getServerDataFile().setData("FullInventory." + entry.getKey() + ".Items." + i, pending.get(i)); + snapshot.set(basePath + ".Items." + i, pending.get(i)); + } + snapshot.set(basePath + ".Time", now); + } + + FileConfiguration data = serverData.getData(); + data.set("FullInventory", null); + ConfigurationSection replacement = snapshot.getConfigurationSection("FullInventory"); + if (replacement != null) { + for (String path : replacement.getKeys(true)) { + if (!replacement.isConfigurationSection(path)) { + data.set("FullInventory." + path, replacement.get(path)); + } } - plugin.getServerDataFile().setData("FullInventory." + entry.getKey() + ".Time", System.currentTimeMillis()); } + serverData.saveData(); } catch (Exception e) { plugin.getLogger().log(Level.WARNING, "Failed to save pending full-inventory items", e); } From 81beca623063f6fadeceb4f8f1d8543ac3f69882 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:16:13 -0600 Subject: [PATCH 13/19] Test single-write full inventory snapshot --- .../tests/item/FullInventoryHandlerTest.java | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java index f08419686d..859f3a1bb4 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -19,6 +20,7 @@ import java.util.concurrent.ScheduledExecutorService; import org.bukkit.Bukkit; +import org.bukkit.configuration.ConfigurationSection; import org.bukkit.configuration.file.YamlConfiguration; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; @@ -139,6 +141,25 @@ public void shutdownStopsOnlyHandlerTimer() { verify(fixture.sharedInventoryTimer, never()).shutdownNow(); } + @Test + public void savePersistsCompletedSnapshotOnce() { + Fixture fixture = createFixture(); + UUID uuid = UUID.randomUUID(); + ItemStack item = mock(ItemStack.class); + fixture.data.set("FullInventory.previous.Time", 123L); + fixture.handler.add(uuid, item); + + fixture.handler.save(); + + verify(fixture.serverData).saveData(); + verify(fixture.serverData, never()).setData(eq("FullInventory"), any()); + ConfigurationSection root = fixture.data.getConfigurationSection("FullInventory"); + assertNotNull(root); + assertFalse(root.contains("previous")); + assertEquals(item, root.getItemStack(uuid + ".Items.0")); + assertTrue(root.getLong(uuid + ".Time") > 0L); + } + private Fixture createFixture() { AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); ScheduledExecutorService sharedInventoryTimer = mock(ScheduledExecutorService.class); @@ -153,20 +174,25 @@ private Fixture createFixture() { FullInventoryHandler handler = new FullInventoryHandler(plugin); handlers.add(handler); - return new Fixture(plugin, sharedInventoryTimer, bukkitScheduler, handler); + return new Fixture(plugin, sharedInventoryTimer, bukkitScheduler, serverData, data, handler); } private static final class Fixture { private final AdvancedCorePlugin plugin; private final ScheduledExecutorService sharedInventoryTimer; private final BukkitScheduler bukkitScheduler; + private final ServerData serverData; + private final YamlConfiguration data; private final FullInventoryHandler handler; private Fixture(AdvancedCorePlugin plugin, ScheduledExecutorService sharedInventoryTimer, - BukkitScheduler bukkitScheduler, FullInventoryHandler handler) { + BukkitScheduler bukkitScheduler, ServerData serverData, YamlConfiguration data, + FullInventoryHandler handler) { this.plugin = plugin; this.sharedInventoryTimer = sharedInventoryTimer; this.bukkitScheduler = bukkitScheduler; + this.serverData = serverData; + this.data = data; this.handler = handler; } } From bd4caf355f4b6ca7a660faf31fc09818ee078701 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:16:48 -0600 Subject: [PATCH 14/19] Document single-save full inventory persistence --- .../api/item/FullInventoryHandler.java | 71 ++----------------- 1 file changed, 4 insertions(+), 67 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index ca6e627bb1..b23dd76002 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -32,61 +32,29 @@ public class FullInventoryHandler { private static final long MESSAGE_COOLDOWN_MS = 5000L; private static final long PENDING_ITEM_RETENTION_MS = TimeUnit.DAYS.toMillis(1); - /** - * The items waiting to be given. - * - * @return the items waiting to be given - */ @Getter private final ConcurrentHashMap> items = new ConcurrentHashMap<>(); private final AdvancedCorePlugin plugin; - /** - * The handler-owned timer executor service. - * - * @return the timer executor service - */ @Getter private ScheduledExecutorService timer; private ScheduledFuture checkTask; - /** - * The last message time for each player. - * - * @return the last message time for each player - */ @Getter private final ConcurrentHashMap lastMessageTime = new ConcurrentHashMap<>(); - /** - * Constructor for FullInventoryHandler. - * - * @param plugin the plugin instance - */ public FullInventoryHandler(AdvancedCorePlugin plugin) { this.plugin = plugin; loadTimer(); startup(); } - /** - * Adds multiple items for a player. - * - * @param uuid the player UUID - * @param item the items to add - */ public void add(UUID uuid, ArrayList item) { addItems(uuid, item); } - /** - * Adds a single item for a player. - * - * @param uuid the player UUID - * @param item the item to add - */ public void add(UUID uuid, ItemStack item) { if (item == null) { return; @@ -96,11 +64,6 @@ public void add(UUID uuid, ItemStack item) { addItems(uuid, itemList); } - /** - * Checks all players for pending items. The sweep itself runs on the normal - * Bukkit/global scheduler, but every player inventory operation is scheduled on - * that player's scheduler/region before it is executed. - */ public void check() { if (!Bukkit.isPrimaryThread()) { plugin.getBukkitScheduler().runTask(plugin, this::schedulePendingPlayerChecks); @@ -109,13 +72,6 @@ public void check() { schedulePendingPlayerChecks(); } - /** - * Checks a specific player for pending items. The inventory work is always - * scheduled through that player's entity scheduler so Folia region ownership is - * respected even when the caller is already on a different tick/region thread. - * - * @param player the player - */ public void check(Player player) { if (player == null) { return; @@ -123,14 +79,6 @@ public void check(Player player) { plugin.getBukkitScheduler().runTask(plugin, () -> checkOwnedPlayer(player), player); } - /** - * Gives items to a player. Inventory/world operations are always scheduled - * through that player's entity scheduler so the public API is safe when called - * from async code, the global region, or another entity's region. - * - * @param player the player - * @param item the items to give - */ public void giveItem(Player player, ItemStack... item) { if (player == null || item == null || item.length == 0) { return; @@ -139,11 +87,6 @@ public void giveItem(Player player, ItemStack... item) { plugin.getBukkitScheduler().runTask(plugin, () -> giveItemOwnedPlayer(player, itemsToGive), player); } - /** - * Loads the timer for checking inventories. The handler keeps its historically - * isolated executor, while actual Bukkit work is handed back to the Bukkit/Folia - * scheduler. - */ public synchronized void loadTimer() { if (timer == null || timer.isShutdown() || timer.isTerminated()) { timer = Executors.newSingleThreadScheduledExecutor(); @@ -157,9 +100,6 @@ public synchronized void loadTimer() { TimeUnit.SECONDS); } - /** - * Stops this handler's repeating task and its handler-owned executor. - */ public synchronized void shutdown() { if (checkTask != null) { checkTask.cancel(false); @@ -171,10 +111,10 @@ public synchronized void shutdown() { } /** - * Saves pending items to disk. The replacement FullInventory section is built in - * a temporary configuration first, then copied into the live configuration and - * persisted in a single save so an incomplete replacement is never deliberately - * written over the previous recovery snapshot. + * Saves pending items to disk. The complete replacement section is first built in + * a temporary configuration. Only after that succeeds is the live configuration + * replaced in memory, followed by a single disk save. This avoids deliberately + * persisting an empty/partial replacement before the new snapshot is complete. */ public void save() { try { @@ -210,9 +150,6 @@ public void save() { } } - /** - * Loads pending items from disk on startup. - */ public void startup() { try { if (plugin.getServerDataFile() == null || plugin.getServerDataFile().getData() == null) { From c1b02de2e21bc75c22c487ed52cf9340bd259a0b Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:18:40 -0600 Subject: [PATCH 15/19] Keep full inventory documentation concise --- .../advancedcore/api/item/FullInventoryHandler.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index b23dd76002..4a577ac842 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -25,9 +25,6 @@ import lombok.Getter; -/** - * Handler for items when player inventories are full. - */ public class FullInventoryHandler { private static final long MESSAGE_COOLDOWN_MS = 5000L; private static final long PENDING_ITEM_RETENTION_MS = TimeUnit.DAYS.toMillis(1); @@ -111,10 +108,9 @@ public synchronized void shutdown() { } /** - * Saves pending items to disk. The complete replacement section is first built in - * a temporary configuration. Only after that succeeds is the live configuration - * replaced in memory, followed by a single disk save. This avoids deliberately - * persisting an empty/partial replacement before the new snapshot is complete. + * Builds the complete replacement section before changing the live config, then + * persists it with one save. This avoids deliberately writing an empty/partial + * recovery snapshot while the replacement is still being constructed. */ public void save() { try { From 1f898aae46b9407225f476b4202a7bf129916abc Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:19:15 -0600 Subject: [PATCH 16/19] Restore focused full inventory documentation --- .../advancedcore/api/item/FullInventoryHandler.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index 4a577ac842..b23dd76002 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -25,6 +25,9 @@ import lombok.Getter; +/** + * Handler for items when player inventories are full. + */ public class FullInventoryHandler { private static final long MESSAGE_COOLDOWN_MS = 5000L; private static final long PENDING_ITEM_RETENTION_MS = TimeUnit.DAYS.toMillis(1); @@ -108,9 +111,10 @@ public synchronized void shutdown() { } /** - * Builds the complete replacement section before changing the live config, then - * persists it with one save. This avoids deliberately writing an empty/partial - * recovery snapshot while the replacement is still being constructed. + * Saves pending items to disk. The complete replacement section is first built in + * a temporary configuration. Only after that succeeds is the live configuration + * replaced in memory, followed by a single disk save. This avoids deliberately + * persisting an empty/partial replacement before the new snapshot is complete. */ public void save() { try { From d5c804f280bd8985ad6459660d185c9a680c0b21 Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:32:29 -0600 Subject: [PATCH 17/19] Quiesce inventory delivery while saving --- .../api/item/FullInventoryHandler.java | 100 +++++++++++------- 1 file changed, 60 insertions(+), 40 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index b23dd76002..3e715002d6 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -10,6 +10,7 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.logging.Level; import org.bukkit.Bukkit; @@ -36,6 +37,7 @@ public class FullInventoryHandler { private final ConcurrentHashMap> items = new ConcurrentHashMap<>(); private final AdvancedCorePlugin plugin; + private final ReentrantReadWriteLock deliveryLock = new ReentrantReadWriteLock(true); @Getter private ScheduledExecutorService timer; @@ -113,10 +115,12 @@ public synchronized void shutdown() { /** * Saves pending items to disk. The complete replacement section is first built in * a temporary configuration. Only after that succeeds is the live configuration - * replaced in memory, followed by a single disk save. This avoids deliberately - * persisting an empty/partial replacement before the new snapshot is complete. + * replaced in memory, followed by a single disk save. Delivery operations hold a + * shared lock, while saving holds the exclusive lock, so a snapshot cannot observe + * the temporary remove/re-add state of an in-flight inventory check. */ public void save() { + deliveryLock.writeLock().lock(); try { ServerData serverData = plugin.getServerDataFile(); if (serverData == null || serverData.getData() == null) { @@ -142,11 +146,12 @@ public void save() { if (!replacement.isConfigurationSection(path)) { data.set("FullInventory." + path, replacement.get(path)); } - } } serverData.saveData(); } catch (Exception e) { plugin.getLogger().log(Level.WARNING, "Failed to save pending full-inventory items", e); + } finally { + deliveryLock.writeLock().unlock(); } } @@ -193,57 +198,72 @@ private void addItems(UUID uuid, Collection itemsToAdd) { if (uuid == null || itemsToAdd == null || itemsToAdd.isEmpty()) { return; } - items.compute(uuid, (key, current) -> { - ArrayList merged = current == null ? new ArrayList<>() : new ArrayList<>(current); - for (ItemStack item : itemsToAdd) { - if (item != null) { - merged.add(item); + deliveryLock.readLock().lock(); + try { + items.compute(uuid, (key, current) -> { + ArrayList merged = current == null ? new ArrayList<>() : new ArrayList<>(current); + for (ItemStack item : itemsToAdd) { + if (item != null) { + merged.add(item); + } } - } - return merged.isEmpty() ? null : merged; - }); + return merged.isEmpty() ? null : merged; + }); + } finally { + deliveryLock.readLock().unlock(); + } } private void checkOwnedPlayer(Player player) { - UUID uuid = player.getUniqueId(); - ArrayList pending = items.remove(uuid); - if (pending == null || pending.isEmpty()) { - return; - } + deliveryLock.readLock().lock(); + try { + UUID uuid = player.getUniqueId(); + ArrayList pending = items.remove(uuid); + if (pending == null || pending.isEmpty()) { + return; + } - ArrayList extra = new ArrayList<>(); - for (ItemStack item : pending) { - if (item == null) { - continue; + ArrayList extra = new ArrayList<>(); + for (ItemStack item : pending) { + if (item == null) { + continue; + } + HashMap excess = player.getInventory().addItem(item); + extra.addAll(excess.values()); } - HashMap excess = player.getInventory().addItem(item); - extra.addAll(excess.values()); - } - if (!extra.isEmpty()) { - addItems(uuid, extra); + if (!extra.isEmpty()) { + addItems(uuid, extra); + } + } finally { + deliveryLock.readLock().unlock(); } } private void giveItemOwnedPlayer(Player player, ItemStack[] item) { - HashMap excess = player.getInventory().addItem(item); - if (excess.isEmpty()) { - player.updateInventory(); - return; - } + deliveryLock.readLock().lock(); + try { + HashMap excess = player.getInventory().addItem(item); + if (excess.isEmpty()) { + player.updateInventory(); + return; + } - boolean dropItems = plugin.getOptions().isDropOnFullInv(); - for (ItemStack extra : excess.values()) { - if (dropItems) { - player.getWorld().dropItem(player.getLocation(), extra); - } else { - add(player.getUniqueId(), extra); + boolean dropItems = plugin.getOptions().isDropOnFullInv(); + for (ItemStack extra : excess.values()) { + if (dropItems) { + player.getWorld().dropItem(player.getLocation(), extra); + } else { + add(player.getUniqueId(), extra); + } } - } - if (shouldSendMessage(player.getUniqueId())) { - sendMessage(player); + if (shouldSendMessage(player.getUniqueId())) { + sendMessage(player); + } + player.updateInventory(); + } finally { + deliveryLock.readLock().unlock(); } - player.updateInventory(); } private void schedulePendingPlayerChecks() { From 362e1649aa8707386e84180f6e42afcc97eb5d9f Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:33:06 -0600 Subject: [PATCH 18/19] Test save quiesces in-flight delivery --- .../tests/item/FullInventoryHandlerTest.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java index 859f3a1bb4..36a345335e 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/item/FullInventoryHandlerTest.java @@ -15,17 +15,22 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.UUID; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import org.bukkit.Bukkit; import org.bukkit.configuration.ConfigurationSection; import org.bukkit.configuration.file.YamlConfiguration; import org.bukkit.entity.Player; import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.mockito.MockedStatic; import com.bencodez.advancedcore.AdvancedCorePlugin; @@ -160,6 +165,54 @@ public void savePersistsCompletedSnapshotOnce() { assertTrue(root.getLong(uuid + ".Time") > 0L); } + @Test + public void saveWaitsForInflightDeliveryBeforeSnapshotting() throws Exception { + Fixture fixture = createFixture(); + UUID uuid = UUID.randomUUID(); + Player player = mock(Player.class); + PlayerInventory inventory = mock(PlayerInventory.class); + ItemStack pending = mock(ItemStack.class); + ItemStack excessItem = mock(ItemStack.class); + CountDownLatch deliveryStarted = new CountDownLatch(1); + CountDownLatch releaseDelivery = new CountDownLatch(1); + CountDownLatch saveFinished = new CountDownLatch(1); + + when(player.getUniqueId()).thenReturn(uuid); + when(player.getInventory()).thenReturn(inventory); + when(inventory.addItem(pending)).thenAnswer(invocation -> { + deliveryStarted.countDown(); + assertTrue(releaseDelivery.await(2, TimeUnit.SECONDS)); + HashMap excess = new HashMap<>(); + excess.put(0, excessItem); + return excess; + }); + fixture.handler.add(uuid, pending); + + fixture.handler.check(player); + ArgumentCaptor deliveryTask = ArgumentCaptor.forClass(Runnable.class); + verify(fixture.bukkitScheduler).runTask(eq(fixture.plugin), deliveryTask.capture(), eq(player)); + + Thread deliveryThread = new Thread(deliveryTask.getValue()); + deliveryThread.start(); + assertTrue(deliveryStarted.await(2, TimeUnit.SECONDS)); + + Thread saveThread = new Thread(() -> { + fixture.handler.save(); + saveFinished.countDown(); + }); + saveThread.start(); + assertFalse(saveFinished.await(100, TimeUnit.MILLISECONDS)); + + releaseDelivery.countDown(); + assertTrue(saveFinished.await(2, TimeUnit.SECONDS)); + deliveryThread.join(2000L); + saveThread.join(2000L); + + ConfigurationSection root = fixture.data.getConfigurationSection("FullInventory"); + assertNotNull(root); + assertEquals(excessItem, root.getItemStack(uuid + ".Items.0")); + } + private Fixture createFixture() { AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class); ScheduledExecutorService sharedInventoryTimer = mock(ScheduledExecutorService.class); From 7d5ac81b2a26f6aac5f0219df78a9ea15f6a667a Mon Sep 17 00:00:00 2001 From: Ben Date: Sun, 23 Aug 2026 20:34:36 -0600 Subject: [PATCH 19/19] Fix save lock syntax --- .../com/bencodez/advancedcore/api/item/FullInventoryHandler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java index 3e715002d6..448e0b14f7 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/item/FullInventoryHandler.java @@ -146,6 +146,7 @@ public void save() { if (!replacement.isConfigurationSection(path)) { data.set("FullInventory." + path, replacement.get(path)); } + } } serverData.saveData(); } catch (Exception e) {