From f9ccbf2516dd29eb2de58bf418db6776ca3e4a4a Mon Sep 17 00:00:00 2001 From: Ben Date: Wed, 23 Sep 2026 18:18:52 -0600 Subject: [PATCH 1/7] Keep cache cleanup off Bukkit state --- .../api/user/usercache/UserDataManager.java | 86 +++++++++++++++++-- .../listeners/PlayerJoinEvent.java | 9 +- ...rDataManagerCacheCleanupThreadingTest.java | 75 ++++++++++++++++ 3 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java index ff22e67d38..8945891c0e 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java @@ -74,6 +74,8 @@ public class UserDataManager { private final Set sharedCachePopulations = ConcurrentHashMap.newKeySet(); private final Set completedSharedCachePopulations = ConcurrentHashMap.newKeySet(); private final ConcurrentHashMap sharedCachePopulationStates = new ConcurrentHashMap<>(); + /** Live storage identities, maintained from platform join/quit events. */ + private final Set onlineUserSessions = ConcurrentHashMap.newKeySet(); private static final class SharedCachePopulationState { private long generation; @@ -1162,19 +1164,91 @@ public void clearCacheBasic() { if (plugin.getStorageType().equals(UserStorage.MYSQL)) plugin.getMysql().clearCacheBasic(); } + /** + * Capture Bukkit-owned online-player state on the platform scheduler, then do + * cache/storage eviction on the user-storage worker. + */ public void clearNonNeededCachedUsers() { + Runnable capture = () -> { + java.util.HashSet online = new java.util.HashSet<>(); + for (Player player : Bukkit.getOnlinePlayers()) { + UUID storageUuid = onlineStorageUuid(player); + if (storageUuid != null) { + online.add(storageUuid); + onlineUserSessions.add(storageUuid); + } + } + try { + timer.execute(() -> { + try { clearNonNeededCachedUsers(online); } + catch (RuntimeException | Error failure) { + reportDeferredStorageFailure(failure); + throw failure; + } + }); + } catch (RejectedExecutionException rejected) { + if (plugin != null && plugin.isEnabled()) reportDeferredStorageFailure(rejected); + } + }; + if (Bukkit.getServer() == null) capture.run(); + else plugin.getBukkitScheduler().runTask(plugin, capture); + } + + private UUID onlineStorageUuid(Player player) { + if (player == null) return null; + if (plugin.getOptions().isOnlineMode()) return player.getUniqueId(); + try { + String resolved = UuidLookup.getInstance().getUUID(player.getName()); + return resolved == null || resolved.isEmpty() ? null : UUID.fromString(resolved); + } catch (RuntimeException failure) { + plugin.debug(failure); + return null; + } + } + + public void markUserOnline(UUID uuid) { + if (uuid != null) onlineUserSessions.add(uuid); + } + + public void markUserOffline(UUID uuid) { + if (uuid != null) onlineUserSessions.remove(uuid); + } + + private void clearNonNeededCachedUsers(Set onlineSnapshot) { plugin.devDebug("Clearing cache for non online players (if any)"); - ArrayList onlineUUIDS = new ArrayList<>(); - for (Player p : Bukkit.getOnlinePlayers()) onlineUUIDS.add(p.getUniqueId()); int removed = 0; - for (UUID uuid : userDataCache.keySet()) { - if (!onlineUUIDS.contains(uuid)) { removeCache(uuid, null); removed++; } + for (UUID uuid : Set.copyOf(userDataCache.keySet())) { + if (onlineSnapshot.contains(uuid) || onlineUserSessions.contains(uuid)) continue; + UserDataCache expected = userDataCache.get(uuid); + if (expected == null) continue; + long expectedVersion = expected.getSharedSnapshotVersion(); + java.util.concurrent.atomic.AtomicBoolean retired = new java.util.concurrent.atomic.AtomicBoolean(); + withSharedSqlBackendExclusive(uuid, () -> { + if (onlineUserSessions.contains(uuid)) return; + UserDataCache current = userDataCache.get(uuid); + if (current != expected || current.getSharedSnapshotVersion() != expectedVersion) return; + current.beginRemoval(); + if (onlineUserSessions.contains(uuid)) { + current.cancelRemoval(); + return; + } + try { + current.clearCache(); + if (sharedSqlRoute != null) current.retireAfterSharedFlush(); + } catch (RuntimeException | Error failure) { + current.cancelRemoval(); + throw failure; + } + boolean cacheRemoved = retireSharedCache(uuid, current); + Consumer listener = sharedCacheRemovalListener; + if (cacheRemoved && listener != null) listener.accept(uuid); + retired.set(cacheRemoved); + }); + if (retired.get()) removed++; } if (removed > 0) plugin.devDebug("Removed " + removed + " cached users who are no longer online"); } - public boolean containsKey(UUID fromString) { return userDataCache.containsKey(fromString); } - /** Return an already-published cache snapshot without creating or populating one. */ public UserDataCache getPublishedCache(UUID uuid) { UserDataCache cache = userDataCache.get(uuid); return cache != null && cache.hasPublishedStorageSnapshot() ? cache : null; diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java index 0f5c094fa5..1092d6c5a6 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java @@ -42,6 +42,8 @@ public void onJoin(AdvancedCoreLoginEvent event) { return; } + try { plugin.getUserManager().getDataManager().markUserOnline(java.util.UUID.fromString(event.getUuid())); } + catch (IllegalArgumentException failure) { plugin.debug(failure); } plugin.getUserManager().getDataManager().cacheUser(player.getUniqueId(), player.getName()); plugin.getBedrockHandle().learn(user); @@ -156,7 +158,12 @@ public void onPlayerQuit(PlayerQuitEvent event) { } Player player = event.getPlayer(); - plugin.debug("Logout: " + player.getName() + " (" + player.getUniqueId() + ")"); + plugin.debug("Logout: " + player.getName() + " (" + player.getUniqueId() + ")"); + try { + java.util.UUID storageUuid = plugin.getOptions().isOnlineMode() ? player.getUniqueId() + : java.util.UUID.fromString(UuidLookup.getInstance().getUUID(player.getName())); + plugin.getUserManager().getDataManager().markUserOffline(storageUuid); + } catch (RuntimeException failure) { plugin.debug(failure); } if (plugin.getPermissionHandler() != null) { plugin.getPermissionHandler().login(player); diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java new file mode 100644 index 0000000000..4fd35701b5 --- /dev/null +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java @@ -0,0 +1,75 @@ +package com.bencodez.advancedcore.tests.user; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.util.UUID; + +import org.bukkit.Bukkit; +import org.bukkit.Server; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; + +import com.bencodez.advancedcore.AdvancedCorePlugin; +import com.bencodez.advancedcore.api.user.usercache.UserDataCache; +import com.bencodez.advancedcore.api.user.usercache.UserDataManager; +import com.bencodez.simpleapi.scheduler.BukkitScheduler; +import com.bencodez.simpleapi.sql.data.DataValueInt; + +class UserDataManagerCacheCleanupThreadingTest { + @Test + void capturesOnlinePlayersOnlyFromPlatformTask() { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + when(plugin.isEnabled()).thenReturn(true); + when(plugin.getOptions().isOnlineMode()).thenReturn(true); + UserDataManager manager = new UserDataManager(plugin); + Server server = mock(Server.class); + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(Bukkit::getOnlinePlayers).thenReturn(java.util.List.of()); + manager.clearNonNeededCachedUsers(); + bukkit.verify(Bukkit::getOnlinePlayers, never()); + ArgumentCaptor platform = ArgumentCaptor.forClass(Runnable.class); + verify(scheduler).runTask(eq(plugin), platform.capture()); + platform.getValue().run(); + bukkit.verify(Bukkit::getOnlinePlayers); + } finally { manager.getTimer().shutdownNow(); } + } + + @Test + void joinAfterPlatformSnapshotPreventsWorkerEviction() throws Exception { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + when(plugin.isEnabled()).thenReturn(true); + when(plugin.getOptions().isOnlineMode()).thenReturn(true); + UserDataManager manager = new UserDataManager(plugin); + manager.getTimer().shutdownNow(); + java.util.concurrent.ScheduledExecutorService worker = mock(java.util.concurrent.ScheduledExecutorService.class); + java.lang.reflect.Field timerField = UserDataManager.class.getDeclaredField("timer"); + timerField.setAccessible(true); + timerField.set(manager, worker); + UUID uuid = UUID.randomUUID(); + UserDataCache cache = new UserDataCache(manager, uuid); + cache.updateCache(new java.util.HashMap<>(java.util.Map.of("Points", new DataValueInt(1)))); + manager.getUserDataCache().put(uuid, cache); + Server server = mock(Server.class); + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(Bukkit::getOnlinePlayers).thenReturn(java.util.List.of()); + ArgumentCaptor platform = ArgumentCaptor.forClass(Runnable.class); + ArgumentCaptor storage = ArgumentCaptor.forClass(Runnable.class); + manager.clearNonNeededCachedUsers(); + verify(scheduler).runTask(eq(plugin), platform.capture()); + platform.getValue().run(); + verify(worker).execute(storage.capture()); + manager.markUserOnline(uuid); + storage.getValue().run(); + assertTrue(manager.containsKey(uuid)); + } + } +} From c93321977bfaede3eb4f80a2bfb04fb1ce329383 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:34:22 -0600 Subject: [PATCH 2/7] Finish worker-owned cache cleanup lifecycle --- .../api/user/usercache/UserDataManager.java | 18 +++++++++++++- .../listeners/PlayerJoinEvent.java | 9 ++----- ...rDataManagerCacheCleanupThreadingTest.java | 24 ++++++++++++++++++- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java index 8945891c0e..dc3c20110e 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java @@ -1191,7 +1191,12 @@ public void clearNonNeededCachedUsers() { } }; if (Bukkit.getServer() == null) capture.run(); - else plugin.getBukkitScheduler().runTask(plugin, capture); + else { + try { plugin.getBukkitScheduler().runTask(plugin, capture); } + catch (RuntimeException failure) { + if (plugin != null && plugin.isEnabled()) reportDeferredStorageFailure(failure); + } + } } private UUID onlineStorageUuid(Player player) { @@ -1210,10 +1215,20 @@ public void markUserOnline(UUID uuid) { if (uuid != null) onlineUserSessions.add(uuid); } + /** Capture an online session from a Bukkit/Folia-owned player event. */ + public void markUserOnline(Player player) { + markUserOnline(onlineStorageUuid(player)); + } + public void markUserOffline(UUID uuid) { if (uuid != null) onlineUserSessions.remove(uuid); } + /** Remove an online session from a Bukkit/Folia-owned player event. */ + public void markUserOffline(Player player) { + markUserOffline(onlineStorageUuid(player)); + } + private void clearNonNeededCachedUsers(Set onlineSnapshot) { plugin.devDebug("Clearing cache for non online players (if any)"); int removed = 0; @@ -1253,6 +1268,7 @@ public UserDataCache getPublishedCache(UUID uuid) { UserDataCache cache = userDataCache.get(uuid); return cache != null && cache.hasPublishedStorageSnapshot() ? cache : null; } + public boolean containsKey(UUID uuid) { return userDataCache.containsKey(uuid); } public UserDataCache getCache(UUID uuid) { if (hasSharedSqlBackend() && Bukkit.getServer() != null && Bukkit.isPrimaryThread()) { UserDataCache cache = userDataCache.get(uuid); diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java index 1092d6c5a6..e82c875df9 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java @@ -42,8 +42,6 @@ public void onJoin(AdvancedCoreLoginEvent event) { return; } - try { plugin.getUserManager().getDataManager().markUserOnline(java.util.UUID.fromString(event.getUuid())); } - catch (IllegalArgumentException failure) { plugin.debug(failure); } plugin.getUserManager().getDataManager().cacheUser(player.getUniqueId(), player.getName()); plugin.getBedrockHandle().learn(user); @@ -71,6 +69,7 @@ public void onPlayerLogin(final org.bukkit.event.player.PlayerJoinEvent event) { } else { plugin.debug("Login: " + event.getPlayer().getName() + " (" + event.getPlayer().getUniqueId() + ")"); } + plugin.getUserManager().getDataManager().markUserOnline(event.getPlayer()); plugin.getLoginTimer().schedule(new Runnable() { @@ -159,11 +158,7 @@ public void onPlayerQuit(PlayerQuitEvent event) { Player player = event.getPlayer(); plugin.debug("Logout: " + player.getName() + " (" + player.getUniqueId() + ")"); - try { - java.util.UUID storageUuid = plugin.getOptions().isOnlineMode() ? player.getUniqueId() - : java.util.UUID.fromString(UuidLookup.getInstance().getUUID(player.getName())); - plugin.getUserManager().getDataManager().markUserOffline(storageUuid); - } catch (RuntimeException failure) { plugin.debug(failure); } + plugin.getUserManager().getDataManager().markUserOffline(player); if (plugin.getPermissionHandler() != null) { plugin.getPermissionHandler().login(player); diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java index 4fd35701b5..6c685d7e7a 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java @@ -1,6 +1,7 @@ package com.bencodez.advancedcore.tests.user; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -19,6 +20,25 @@ import com.bencodez.simpleapi.sql.data.DataValueInt; class UserDataManagerCacheCleanupThreadingTest { + @Test + void platformSchedulerRejectionDoesNotStopLaterCleanupAttempts() { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + when(plugin.isEnabled()).thenReturn(true); + doThrow(new IllegalStateException("scheduler unavailable")) + .doNothing().when(scheduler).runTask(eq(plugin), any(Runnable.class)); + UserDataManager manager = new UserDataManager(plugin); + Server server = mock(Server.class); + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::getServer).thenReturn(server); + assertDoesNotThrow(manager::clearNonNeededCachedUsers); + assertDoesNotThrow(manager::clearNonNeededCachedUsers); + verify(scheduler, times(2)).runTask(eq(plugin), any(Runnable.class)); + assertTrue(manager.getLastDeferredStorageFailure() instanceof IllegalStateException); + } finally { manager.getTimer().shutdownNow(); } + } + @Test void capturesOnlinePlayersOnlyFromPlatformTask() { AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); @@ -67,7 +87,9 @@ void joinAfterPlatformSnapshotPreventsWorkerEviction() throws Exception { verify(scheduler).runTask(eq(plugin), platform.capture()); platform.getValue().run(); verify(worker).execute(storage.capture()); - manager.markUserOnline(uuid); + org.bukkit.entity.Player player = mock(org.bukkit.entity.Player.class); + when(player.getUniqueId()).thenReturn(uuid); + manager.markUserOnline(player); storage.getValue().run(); assertTrue(manager.containsKey(uuid)); } From bf11544ebb46e05aa86dca77a2173b702964fd0e Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:50:09 -0600 Subject: [PATCH 3/7] Fence cache snapshots against concurrent quits --- .../api/user/usercache/UserDataManager.java | 30 ++++++++++------ ...rDataManagerCacheCleanupThreadingTest.java | 36 +++++++++++++++++++ 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java index dc3c20110e..eef66008ab 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java @@ -75,7 +75,10 @@ public class UserDataManager { private final Set completedSharedCachePopulations = ConcurrentHashMap.newKeySet(); private final ConcurrentHashMap sharedCachePopulationStates = new ConcurrentHashMap<>(); /** Live storage identities, maintained from platform join/quit events. */ - private final Set onlineUserSessions = ConcurrentHashMap.newKeySet(); + // TRUE/FALSE records the latest player-event state. Offline tombstones keep a + // concurrent Folia snapshot from re-adding a player whose quit event won the + // race; a later snapshot removes tombstones once Bukkit no longer lists them. + private final ConcurrentHashMap onlineUserSessions = new ConcurrentHashMap<>(); private static final class SharedCachePopulationState { private long generation; @@ -1170,14 +1173,19 @@ public void clearCacheBasic() { */ public void clearNonNeededCachedUsers() { Runnable capture = () -> { - java.util.HashSet online = new java.util.HashSet<>(); + java.util.HashSet platformOnline = new java.util.HashSet<>(); for (Player player : Bukkit.getOnlinePlayers()) { UUID storageUuid = onlineStorageUuid(player); - if (storageUuid != null) { - online.add(storageUuid); - onlineUserSessions.add(storageUuid); - } + if (storageUuid != null) platformOnline.add(storageUuid); + } + java.util.HashSet online = new java.util.HashSet<>(); + for (UUID uuid : platformOnline) { + Boolean state = onlineUserSessions.compute(uuid, + (ignored, current) -> Boolean.FALSE.equals(current) ? Boolean.FALSE : Boolean.TRUE); + if (Boolean.TRUE.equals(state)) online.add(uuid); } + onlineUserSessions.entrySet().removeIf(entry -> Boolean.FALSE.equals(entry.getValue()) + && !platformOnline.contains(entry.getKey())); try { timer.execute(() -> { try { clearNonNeededCachedUsers(online); } @@ -1212,7 +1220,7 @@ private UUID onlineStorageUuid(Player player) { } public void markUserOnline(UUID uuid) { - if (uuid != null) onlineUserSessions.add(uuid); + if (uuid != null) onlineUserSessions.put(uuid, Boolean.TRUE); } /** Capture an online session from a Bukkit/Folia-owned player event. */ @@ -1221,7 +1229,7 @@ public void markUserOnline(Player player) { } public void markUserOffline(UUID uuid) { - if (uuid != null) onlineUserSessions.remove(uuid); + if (uuid != null) onlineUserSessions.put(uuid, Boolean.FALSE); } /** Remove an online session from a Bukkit/Folia-owned player event. */ @@ -1233,17 +1241,17 @@ private void clearNonNeededCachedUsers(Set onlineSnapshot) { plugin.devDebug("Clearing cache for non online players (if any)"); int removed = 0; for (UUID uuid : Set.copyOf(userDataCache.keySet())) { - if (onlineSnapshot.contains(uuid) || onlineUserSessions.contains(uuid)) continue; + if (onlineSnapshot.contains(uuid) || Boolean.TRUE.equals(onlineUserSessions.get(uuid))) continue; UserDataCache expected = userDataCache.get(uuid); if (expected == null) continue; long expectedVersion = expected.getSharedSnapshotVersion(); java.util.concurrent.atomic.AtomicBoolean retired = new java.util.concurrent.atomic.AtomicBoolean(); withSharedSqlBackendExclusive(uuid, () -> { - if (onlineUserSessions.contains(uuid)) return; + if (Boolean.TRUE.equals(onlineUserSessions.get(uuid))) return; UserDataCache current = userDataCache.get(uuid); if (current != expected || current.getSharedSnapshotVersion() != expectedVersion) return; current.beginRemoval(); - if (onlineUserSessions.contains(uuid)) { + if (Boolean.TRUE.equals(onlineUserSessions.get(uuid))) { current.cancelRemoval(); return; } diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java index 6c685d7e7a..d6a64ece03 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java @@ -94,4 +94,40 @@ void joinAfterPlatformSnapshotPreventsWorkerEviction() throws Exception { assertTrue(manager.containsKey(uuid)); } } + + @Test + void concurrentQuitCannotBeOverwrittenByPlatformSnapshot() throws Exception { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + when(plugin.isEnabled()).thenReturn(true); + when(plugin.getOptions().isOnlineMode()).thenReturn(true); + UserDataManager manager = new UserDataManager(plugin); + manager.getTimer().shutdownNow(); + java.util.concurrent.ScheduledExecutorService worker = mock(java.util.concurrent.ScheduledExecutorService.class); + java.lang.reflect.Field timerField = UserDataManager.class.getDeclaredField("timer"); + timerField.setAccessible(true); + timerField.set(manager, worker); + UUID uuid = UUID.randomUUID(); + UserDataCache cache = new UserDataCache(manager, uuid); + cache.updateCache(new java.util.HashMap<>(java.util.Map.of("Points", new DataValueInt(1)))); + manager.getUserDataCache().put(uuid, cache); + org.bukkit.entity.Player player = mock(org.bukkit.entity.Player.class); + when(player.getUniqueId()).thenReturn(uuid); + Server server = mock(Server.class); + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(Bukkit::getOnlinePlayers).thenReturn(java.util.List.of(player)); + ArgumentCaptor platform = ArgumentCaptor.forClass(Runnable.class); + ArgumentCaptor storage = ArgumentCaptor.forClass(Runnable.class); + manager.markUserOnline(player); + manager.clearNonNeededCachedUsers(); + verify(scheduler).runTask(eq(plugin), platform.capture()); + manager.markUserOffline(player); + platform.getValue().run(); + verify(worker).execute(storage.capture()); + storage.getValue().run(); + assertTrue(!manager.containsKey(uuid)); + } + } } From fe461c6edd22e42c64ea5fc93a40f847ee59550b Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 23 Sep 2026 19:20:17 -0600 Subject: [PATCH 4/7] Fence cache retirement against player joins --- .../api/user/usercache/UserDataManager.java | 52 ++++++++++++++----- ...rDataManagerCacheCleanupThreadingTest.java | 34 ++++++++++++ 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java index eef66008ab..9cfc0aa094 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java @@ -79,6 +79,17 @@ public class UserDataManager { // concurrent Folia snapshot from re-adding a player whose quit event won the // race; a later snapshot removes tombstones once Bukkit no longer lists them. private final ConcurrentHashMap onlineUserSessions = new ConcurrentHashMap<>(); + private final Object[] onlineSessionLocks = createOnlineSessionLocks(); + + private static Object[] createOnlineSessionLocks() { + Object[] locks = new Object[64]; + java.util.Arrays.setAll(locks, ignored -> new Object()); + return locks; + } + + private Object onlineSessionLock(UUID uuid) { + return onlineSessionLocks[(uuid.hashCode() & Integer.MAX_VALUE) % onlineSessionLocks.length]; + } private static final class SharedCachePopulationState { private long generation; @@ -1180,12 +1191,19 @@ public void clearNonNeededCachedUsers() { } java.util.HashSet online = new java.util.HashSet<>(); for (UUID uuid : platformOnline) { - Boolean state = onlineUserSessions.compute(uuid, - (ignored, current) -> Boolean.FALSE.equals(current) ? Boolean.FALSE : Boolean.TRUE); - if (Boolean.TRUE.equals(state)) online.add(uuid); + synchronized (onlineSessionLock(uuid)) { + Boolean state = onlineUserSessions.compute(uuid, + (ignored, current) -> Boolean.FALSE.equals(current) ? Boolean.FALSE : Boolean.TRUE); + if (Boolean.TRUE.equals(state)) online.add(uuid); + } + } + for (UUID uuid : Set.copyOf(onlineUserSessions.keySet())) { + synchronized (onlineSessionLock(uuid)) { + if (Boolean.FALSE.equals(onlineUserSessions.get(uuid)) && !platformOnline.contains(uuid)) { + onlineUserSessions.remove(uuid, Boolean.FALSE); + } + } } - onlineUserSessions.entrySet().removeIf(entry -> Boolean.FALSE.equals(entry.getValue()) - && !platformOnline.contains(entry.getKey())); try { timer.execute(() -> { try { clearNonNeededCachedUsers(online); } @@ -1220,7 +1238,9 @@ private UUID onlineStorageUuid(Player player) { } public void markUserOnline(UUID uuid) { - if (uuid != null) onlineUserSessions.put(uuid, Boolean.TRUE); + if (uuid != null) synchronized (onlineSessionLock(uuid)) { + onlineUserSessions.put(uuid, Boolean.TRUE); + } } /** Capture an online session from a Bukkit/Folia-owned player event. */ @@ -1229,7 +1249,9 @@ public void markUserOnline(Player player) { } public void markUserOffline(UUID uuid) { - if (uuid != null) onlineUserSessions.put(uuid, Boolean.FALSE); + if (uuid != null) synchronized (onlineSessionLock(uuid)) { + onlineUserSessions.put(uuid, Boolean.FALSE); + } } /** Remove an online session from a Bukkit/Folia-owned player event. */ @@ -1257,16 +1279,22 @@ private void clearNonNeededCachedUsers(Set onlineSnapshot) { } try { current.clearCache(); - if (sharedSqlRoute != null) current.retireAfterSharedFlush(); } catch (RuntimeException | Error failure) { current.cancelRemoval(); throw failure; } - boolean cacheRemoved = retireSharedCache(uuid, current); - Consumer listener = sharedCacheRemovalListener; - if (cacheRemoved && listener != null) listener.accept(uuid); - retired.set(cacheRemoved); + synchronized (onlineSessionLock(uuid)) { + if (Boolean.TRUE.equals(onlineUserSessions.get(uuid))) { + current.cancelRemoval(); + return; + } + if (sharedSqlRoute != null) current.retireAfterSharedFlush(); + boolean cacheRemoved = retireSharedCache(uuid, current); + retired.set(cacheRemoved); + } }); + Consumer listener = sharedCacheRemovalListener; + if (retired.get() && listener != null) listener.accept(uuid); if (retired.get()) removed++; } if (removed > 0) plugin.devDebug("Removed " + removed + " cached users who are no longer online"); diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java index d6a64ece03..f9dd6a8a6d 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java @@ -6,6 +6,8 @@ import static org.mockito.Mockito.*; import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import org.bukkit.Bukkit; import org.bukkit.Server; @@ -130,4 +132,36 @@ void concurrentQuitCannotBeOverwrittenByPlatformSnapshot() throws Exception { assertTrue(!manager.containsKey(uuid)); } } + + @Test + void joinDuringBlockedFlushPreventsRetirementWithoutBlockingJoin() throws Exception { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); + when(plugin.isEnabled()).thenReturn(true); + UserDataManager manager = new UserDataManager(plugin); + UUID uuid = UUID.randomUUID(); + UserDataCache cache = mock(UserDataCache.class); + when(cache.getSharedSnapshotVersion()).thenReturn(1L); + CountDownLatch flushStarted = new CountDownLatch(1); + CountDownLatch releaseFlush = new CountDownLatch(1); + doAnswer(invocation -> { + flushStarted.countDown(); + assertTrue(releaseFlush.await(5, TimeUnit.SECONDS)); + return null; + }).when(cache).clearCache(); + manager.getUserDataCache().put(uuid, cache); + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::getServer).thenReturn(null); + manager.clearNonNeededCachedUsers(); + assertTrue(flushStarted.await(5, TimeUnit.SECONDS)); + long started = System.nanoTime(); + manager.markUserOnline(uuid); + assertTrue(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - started) < 500); + releaseFlush.countDown(); + manager.getTimer().shutdown(); + assertTrue(manager.getTimer().awaitTermination(5, TimeUnit.SECONDS)); + assertTrue(manager.containsKey(uuid)); + verify(cache).cancelRemoval(); + verify(cache, never()).retireAfterSharedFlush(); + } finally { manager.getTimer().shutdownNow(); } + } } From c092125a4dc5d009651124d668d36ce813e4bc77 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 23 Sep 2026 20:36:59 -0600 Subject: [PATCH 5/7] Invalidate cleared caches after concurrent joins --- .../api/user/usercache/UserDataManager.java | 14 ++++++++++++-- .../advancedcore/listeners/PlayerJoinEvent.java | 12 ++++++------ .../listeners/PlayerJoinEventSessionTest.java | 15 +++++++++++++++ .../UserDataManagerCacheCleanupThreadingTest.java | 7 ++++--- 4 files changed, 37 insertions(+), 11 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java index 61d7fddb11..77d8b61995 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java @@ -1270,6 +1270,7 @@ private void clearNonNeededCachedUsers(Set onlineSnapshot) { if (expected == null) continue; long expectedVersion = expected.getSharedSnapshotVersion(); java.util.concurrent.atomic.AtomicBoolean retired = new java.util.concurrent.atomic.AtomicBoolean(); + java.util.concurrent.atomic.AtomicBoolean invalidatedForJoin = new java.util.concurrent.atomic.AtomicBoolean(); withSharedSqlBackendExclusive(uuid, () -> { if (Boolean.TRUE.equals(onlineUserSessions.get(uuid))) return; UserDataCache current = userDataCache.get(uuid); @@ -1287,7 +1288,16 @@ private void clearNonNeededCachedUsers(Set onlineSnapshot) { } synchronized (onlineSessionLock(uuid)) { if (Boolean.TRUE.equals(onlineUserSessions.get(uuid))) { - current.cancelRemoval(); + if (sharedSqlRoute != null) { + try { current.retireAfterSharedFlush(); } + catch (RuntimeException | Error failure) { + current.cancelRemoval(); + throw failure; + } + } + boolean cacheRemoved = retireSharedCache(uuid, current); + retired.set(cacheRemoved); + invalidatedForJoin.set(cacheRemoved); return; } if (sharedSqlRoute != null) { @@ -1303,7 +1313,7 @@ private void clearNonNeededCachedUsers(Set onlineSnapshot) { }); Consumer listener = sharedCacheRemovalListener; if (retired.get() && listener != null) listener.accept(uuid); - if (retired.get()) removed++; + if (retired.get() && !invalidatedForJoin.get()) removed++; } if (removed > 0) plugin.devDebug("Removed " + removed + " cached users who are no longer online"); } diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java index f73e19dc9f..0f9aac5fa1 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/listeners/PlayerJoinEvent.java @@ -65,11 +65,13 @@ public void onJoin(AdvancedCoreLoginEvent event) { } } - @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) public void onPlayerLogin(final org.bukkit.event.player.PlayerJoinEvent event) { - if (plugin == null || !plugin.isEnabled() || !plugin.isLoadUserData()) { - return; - } + if (plugin == null || !plugin.isEnabled()) { + return; + } + plugin.getUserManager().getDataManager().markUserOnline(event.getPlayer()); + if (!plugin.isLoadUserData()) return; if (!plugin.getOptions().isHideLoginMessage()) { plugin.getLogger() @@ -77,8 +79,6 @@ public void onPlayerLogin(final org.bukkit.event.player.PlayerJoinEvent event) { } else { plugin.debug("Login: " + event.getPlayer().getName() + " (" + event.getPlayer().getUniqueId() + ")"); } - plugin.getUserManager().getDataManager().markUserOnline(event.getPlayer()); - Player joiningPlayer = event.getPlayer(); synchronized (loginSessionLock(joiningPlayer.getUniqueId())) { pendingLoginSessions.put(joiningPlayer.getUniqueId(), joiningPlayer); diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/listeners/PlayerJoinEventSessionTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/listeners/PlayerJoinEventSessionTest.java index a019453273..66550011fa 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/listeners/PlayerJoinEventSessionTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/listeners/PlayerJoinEventSessionTest.java @@ -15,6 +15,21 @@ import com.bencodez.advancedcore.api.permissions.PermissionHandler; class PlayerJoinEventSessionTest { + @Test + void joinTracksOnlineStateWhenUserDataLoadingIsDisabled() { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); + when(plugin.isEnabled()).thenReturn(true); + when(plugin.isLoadUserData()).thenReturn(false); + Player player = mock(Player.class); + org.bukkit.event.player.PlayerJoinEvent join = mock(org.bukkit.event.player.PlayerJoinEvent.class); + when(join.getPlayer()).thenReturn(player); + + new PlayerJoinEvent(plugin).onPlayerLogin(join); + + verify(plugin.getUserManager().getDataManager()).markUserOnline(player); + verify(plugin.getLoginTimer(), never()).schedule(any(Runnable.class), anyLong(), any(TimeUnit.class)); + } + @Test void delayedLoginDoesNotRestorePermissionsAfterThePlayerQuit() { AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java index 4285fbfa8d..dd103a4ca9 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -181,7 +182,7 @@ void concurrentQuitCannotBeOverwrittenByPlatformSnapshot() throws Exception { } @Test - void joinDuringBlockedFlushPreventsRetirementWithoutBlockingJoin() throws Exception { + void joinDuringBlockedFlushInvalidatesClearedCacheWithoutBlockingJoin() throws Exception { AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); when(plugin.isEnabled()).thenReturn(true); UserDataManager manager = new UserDataManager(plugin); @@ -206,8 +207,8 @@ void joinDuringBlockedFlushPreventsRetirementWithoutBlockingJoin() throws Except releaseFlush.countDown(); manager.getTimer().shutdown(); assertTrue(manager.getTimer().awaitTermination(5, TimeUnit.SECONDS)); - assertTrue(manager.containsKey(uuid)); - verify(cache).cancelRemoval(); + assertFalse(manager.containsKey(uuid)); + verify(cache, never()).cancelRemoval(); verify(cache, never()).retireAfterSharedFlush(); } finally { manager.getTimer().shutdownNow(); } } From ca9af6c77143429c92a845210b0d5b53848e1799 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 23 Sep 2026 20:42:54 -0600 Subject: [PATCH 6/7] Move session reconciliation to storage worker --- .../api/user/usercache/UserDataManager.java | 36 ++++++++++--------- ...rDataManagerCacheCleanupThreadingTest.java | 36 +++++++++++++++++++ 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java index 77d8b61995..2957213303 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java @@ -1191,24 +1191,9 @@ public void clearNonNeededCachedUsers() { if (storageUuid != null) platformOnline.add(storageUuid); } } - java.util.HashSet online = new java.util.HashSet<>(); - for (UUID uuid : platformOnline) { - synchronized (onlineSessionLock(uuid)) { - Boolean state = onlineUserSessions.compute(uuid, - (ignored, current) -> Boolean.FALSE.equals(current) ? Boolean.FALSE : Boolean.TRUE); - if (Boolean.TRUE.equals(state)) online.add(uuid); - } - } - for (UUID uuid : Set.copyOf(onlineUserSessions.keySet())) { - synchronized (onlineSessionLock(uuid)) { - if (Boolean.FALSE.equals(onlineUserSessions.get(uuid)) && !platformOnline.contains(uuid)) { - onlineUserSessions.remove(uuid, Boolean.FALSE); - } - } - } try { timer.execute(() -> { - try { clearNonNeededCachedUsers(online); } + try { clearNonNeededCachedUsers(reconcileOnlineSnapshot(platformOnline)); } catch (RuntimeException | Error failure) { reportDeferredStorageFailure(failure); throw failure; @@ -1227,6 +1212,25 @@ public void clearNonNeededCachedUsers() { } } + private Set reconcileOnlineSnapshot(Set platformOnline) { + java.util.HashSet online = new java.util.HashSet<>(); + for (UUID uuid : platformOnline) { + synchronized (onlineSessionLock(uuid)) { + Boolean state = onlineUserSessions.compute(uuid, + (ignored, current) -> Boolean.FALSE.equals(current) ? Boolean.FALSE : Boolean.TRUE); + if (Boolean.TRUE.equals(state)) online.add(uuid); + } + } + for (UUID uuid : Set.copyOf(onlineUserSessions.keySet())) { + synchronized (onlineSessionLock(uuid)) { + if (Boolean.FALSE.equals(onlineUserSessions.get(uuid)) && !platformOnline.contains(uuid)) { + onlineUserSessions.remove(uuid, Boolean.FALSE); + } + } + } + return online; + } + private UUID onlineStorageUuid(Player player) { if (player == null) return null; if (plugin.getOptions().isOnlineMode()) return player.getUniqueId(); diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java index dd103a4ca9..6489c65c59 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java @@ -9,6 +9,7 @@ import java.util.UUID; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import org.bukkit.Bukkit; @@ -110,6 +111,41 @@ void capturesOnlinePlayersOnlyFromPlatformTask() { } finally { manager.getTimer().shutdownNow(); } } + @Test + void reconcilesSessionMarkersOnlyOnStorageWorker() throws Exception { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + when(plugin.isEnabled()).thenReturn(true); + UserDataManager manager = new UserDataManager(plugin); + manager.getTimer().shutdownNow(); + java.util.concurrent.ScheduledExecutorService worker = mock(java.util.concurrent.ScheduledExecutorService.class); + java.lang.reflect.Field timerField = UserDataManager.class.getDeclaredField("timer"); + timerField.setAccessible(true); + timerField.set(manager, worker); + UUID uuid = UUID.randomUUID(); + manager.markUserOffline(uuid); + java.lang.reflect.Field sessionsField = UserDataManager.class.getDeclaredField("onlineUserSessions"); + sessionsField.setAccessible(true); + @SuppressWarnings("unchecked") + ConcurrentHashMap sessions = + (ConcurrentHashMap) sessionsField.get(manager); + Server server = mock(Server.class); + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(Bukkit::getOnlinePlayers).thenReturn(java.util.List.of()); + ArgumentCaptor platform = ArgumentCaptor.forClass(Runnable.class); + ArgumentCaptor storage = ArgumentCaptor.forClass(Runnable.class); + manager.clearNonNeededCachedUsers(); + verify(scheduler).runTask(eq(plugin), platform.capture()); + platform.getValue().run(); + assertTrue(sessions.containsKey(uuid)); + verify(worker).execute(storage.capture()); + storage.getValue().run(); + assertFalse(sessions.containsKey(uuid)); + } + } + @Test void joinAfterPlatformSnapshotPreventsWorkerEviction() throws Exception { AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); From 5fd805d2301be7330a7014702bcc35c8b35f23c4 Mon Sep 17 00:00:00 2001 From: BenCodez <17074231+BenCodez@users.noreply.github.com> Date: Wed, 23 Sep 2026 20:50:34 -0600 Subject: [PATCH 7/7] Fence queued cache snapshots by session generation --- .../api/user/usercache/UserDataManager.java | 45 ++++++++++------- ...rDataManagerCacheCleanupThreadingTest.java | 49 ++++++++++++++++++- 2 files changed, 75 insertions(+), 19 deletions(-) diff --git a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java index 2957213303..69da17a1d2 100644 --- a/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java +++ b/AdvancedCore/src/main/java/com/bencodez/advancedcore/api/user/usercache/UserDataManager.java @@ -75,11 +75,13 @@ public class UserDataManager { private final Set completedSharedCachePopulations = ConcurrentHashMap.newKeySet(); private final ConcurrentHashMap sharedCachePopulationStates = new ConcurrentHashMap<>(); /** Live storage identities, maintained from platform join/quit events. */ - // TRUE/FALSE records the latest player-event state. Offline tombstones keep a - // concurrent Folia snapshot from re-adding a player whose quit event won the - // race; a later snapshot removes tombstones once Bukkit no longer lists them. - private final ConcurrentHashMap onlineUserSessions = new ConcurrentHashMap<>(); + // Generation-stamped offline tombstones prevent an older queued platform + // snapshot from erasing a newer quit and letting a later stale snapshot + // resurrect that session. + private final ConcurrentHashMap onlineUserSessions = new ConcurrentHashMap<>(); + private final AtomicLong onlineSessionGeneration = new AtomicLong(); private final Object[] onlineSessionLocks = createOnlineSessionLocks(); + private record OnlineSessionState(boolean online, long generation) {} private static Object[] createOnlineSessionLocks() { Object[] locks = new Object[64]; @@ -1191,9 +1193,10 @@ public void clearNonNeededCachedUsers() { if (storageUuid != null) platformOnline.add(storageUuid); } } + long snapshotGeneration = onlineSessionGeneration.get(); try { timer.execute(() -> { - try { clearNonNeededCachedUsers(reconcileOnlineSnapshot(platformOnline)); } + try { clearNonNeededCachedUsers(reconcileOnlineSnapshot(platformOnline, snapshotGeneration)); } catch (RuntimeException | Error failure) { reportDeferredStorageFailure(failure); throw failure; @@ -1212,19 +1215,22 @@ public void clearNonNeededCachedUsers() { } } - private Set reconcileOnlineSnapshot(Set platformOnline) { + private Set reconcileOnlineSnapshot(Set platformOnline, long snapshotGeneration) { java.util.HashSet online = new java.util.HashSet<>(); for (UUID uuid : platformOnline) { synchronized (onlineSessionLock(uuid)) { - Boolean state = onlineUserSessions.compute(uuid, - (ignored, current) -> Boolean.FALSE.equals(current) ? Boolean.FALSE : Boolean.TRUE); - if (Boolean.TRUE.equals(state)) online.add(uuid); + OnlineSessionState state = onlineUserSessions.compute(uuid, + (ignored, current) -> current == null + ? new OnlineSessionState(true, snapshotGeneration) : current); + if (state.online()) online.add(uuid); } } for (UUID uuid : Set.copyOf(onlineUserSessions.keySet())) { synchronized (onlineSessionLock(uuid)) { - if (Boolean.FALSE.equals(onlineUserSessions.get(uuid)) && !platformOnline.contains(uuid)) { - onlineUserSessions.remove(uuid, Boolean.FALSE); + OnlineSessionState state = onlineUserSessions.get(uuid); + if (state != null && !state.online() && state.generation() <= snapshotGeneration + && !platformOnline.contains(uuid)) { + onlineUserSessions.remove(uuid, state); } } } @@ -1245,7 +1251,7 @@ private UUID onlineStorageUuid(Player player) { public void markUserOnline(UUID uuid) { if (uuid != null) synchronized (onlineSessionLock(uuid)) { - onlineUserSessions.put(uuid, Boolean.TRUE); + onlineUserSessions.put(uuid, new OnlineSessionState(true, onlineSessionGeneration.incrementAndGet())); } } @@ -1256,7 +1262,7 @@ public void markUserOnline(Player player) { public void markUserOffline(UUID uuid) { if (uuid != null) synchronized (onlineSessionLock(uuid)) { - onlineUserSessions.put(uuid, Boolean.FALSE); + onlineUserSessions.put(uuid, new OnlineSessionState(false, onlineSessionGeneration.incrementAndGet())); } } @@ -1269,18 +1275,18 @@ private void clearNonNeededCachedUsers(Set onlineSnapshot) { plugin.devDebug("Clearing cache for non online players (if any)"); int removed = 0; for (UUID uuid : Set.copyOf(userDataCache.keySet())) { - if (onlineSnapshot.contains(uuid) || Boolean.TRUE.equals(onlineUserSessions.get(uuid))) continue; + if (onlineSnapshot.contains(uuid) || isUserOnline(uuid)) continue; UserDataCache expected = userDataCache.get(uuid); if (expected == null) continue; long expectedVersion = expected.getSharedSnapshotVersion(); java.util.concurrent.atomic.AtomicBoolean retired = new java.util.concurrent.atomic.AtomicBoolean(); java.util.concurrent.atomic.AtomicBoolean invalidatedForJoin = new java.util.concurrent.atomic.AtomicBoolean(); withSharedSqlBackendExclusive(uuid, () -> { - if (Boolean.TRUE.equals(onlineUserSessions.get(uuid))) return; + if (isUserOnline(uuid)) return; UserDataCache current = userDataCache.get(uuid); if (current != expected || current.getSharedSnapshotVersion() != expectedVersion) return; current.beginRemoval(); - if (Boolean.TRUE.equals(onlineUserSessions.get(uuid))) { + if (isUserOnline(uuid)) { current.cancelRemoval(); return; } @@ -1291,7 +1297,7 @@ private void clearNonNeededCachedUsers(Set onlineSnapshot) { throw failure; } synchronized (onlineSessionLock(uuid)) { - if (Boolean.TRUE.equals(onlineUserSessions.get(uuid))) { + if (isUserOnline(uuid)) { if (sharedSqlRoute != null) { try { current.retireAfterSharedFlush(); } catch (RuntimeException | Error failure) { @@ -1322,6 +1328,11 @@ private void clearNonNeededCachedUsers(Set onlineSnapshot) { if (removed > 0) plugin.devDebug("Removed " + removed + " cached users who are no longer online"); } + private boolean isUserOnline(UUID uuid) { + OnlineSessionState state = onlineUserSessions.get(uuid); + return state != null && state.online(); + } + public UserDataCache getPublishedCache(UUID uuid) { UserDataCache cache = userDataCache.get(uuid); return cache != null && cache.hasPublishedStorageSnapshot() ? cache : null; diff --git a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java index 6489c65c59..9f3609fe90 100644 --- a/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java +++ b/AdvancedCore/src/test/java/com/bencodez/advancedcore/tests/user/UserDataManagerCacheCleanupThreadingTest.java @@ -128,8 +128,7 @@ void reconcilesSessionMarkersOnlyOnStorageWorker() throws Exception { java.lang.reflect.Field sessionsField = UserDataManager.class.getDeclaredField("onlineUserSessions"); sessionsField.setAccessible(true); @SuppressWarnings("unchecked") - ConcurrentHashMap sessions = - (ConcurrentHashMap) sessionsField.get(manager); + ConcurrentHashMap sessions = (ConcurrentHashMap) sessionsField.get(manager); Server server = mock(Server.class); try (MockedStatic bukkit = mockStatic(Bukkit.class)) { bukkit.when(Bukkit::getServer).thenReturn(server); @@ -146,6 +145,52 @@ void reconcilesSessionMarkersOnlyOnStorageWorker() throws Exception { } } + @Test + void olderQueuedSnapshotCannotEraseNewerQuitTombstone() throws Exception { + AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS); + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(plugin.getBukkitScheduler()).thenReturn(scheduler); + when(plugin.isEnabled()).thenReturn(true); + when(plugin.getOptions().isOnlineMode()).thenReturn(true); + UserDataManager manager = new UserDataManager(plugin); + manager.getTimer().shutdownNow(); + java.util.concurrent.ScheduledExecutorService worker = mock(java.util.concurrent.ScheduledExecutorService.class); + java.lang.reflect.Field timerField = UserDataManager.class.getDeclaredField("timer"); + timerField.setAccessible(true); + timerField.set(manager, worker); + UUID uuid = UUID.randomUUID(); + org.bukkit.entity.Player player = mock(org.bukkit.entity.Player.class); + when(player.getUniqueId()).thenReturn(uuid); + Server server = mock(Server.class); + try (MockedStatic bukkit = mockStatic(Bukkit.class)) { + bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(Bukkit::getOnlinePlayers) + .thenReturn(java.util.List.of(), java.util.List.of(player), java.util.List.of()); + ArgumentCaptor platform = ArgumentCaptor.forClass(Runnable.class); + ArgumentCaptor storage = ArgumentCaptor.forClass(Runnable.class); + + manager.clearNonNeededCachedUsers(); + manager.clearNonNeededCachedUsers(); + verify(scheduler, times(2)).runTask(eq(plugin), platform.capture()); + platform.getAllValues().forEach(Runnable::run); + verify(worker, times(2)).execute(storage.capture()); + manager.markUserOffline(player); + storage.getAllValues().forEach(Runnable::run); + + UserDataCache cache = new UserDataCache(manager, uuid); + cache.updateCache(new java.util.HashMap<>(java.util.Map.of("Points", new DataValueInt(1)))); + manager.getUserDataCache().put(uuid, cache); + manager.clearNonNeededCachedUsers(); + ArgumentCaptor allPlatformTasks = ArgumentCaptor.forClass(Runnable.class); + verify(scheduler, times(3)).runTask(eq(plugin), allPlatformTasks.capture()); + allPlatformTasks.getAllValues().get(2).run(); + ArgumentCaptor allStorageTasks = ArgumentCaptor.forClass(Runnable.class); + verify(worker, times(3)).execute(allStorageTasks.capture()); + allStorageTasks.getAllValues().get(2).run(); + assertFalse(manager.containsKey(uuid)); + } + } + @Test void joinAfterPlatformSnapshotPreventsWorkerEviction() throws Exception { AdvancedCorePlugin plugin = mock(AdvancedCorePlugin.class, RETURNS_DEEP_STUBS);