From 229a01d86cc7b9f8e6cdb3c8c50254cb1e8ad9a2 Mon Sep 17 00:00:00 2001 From: Arbousier1 Date: Tue, 30 Jun 2026 08:28:14 +0000 Subject: [PATCH] fix: harden component storage consistency --- .../java/com/fastsync/data/PlayerData.java | 8 ++ .../fastsync/database/DatabaseManager.java | 3 +- .../serialization/PlayerDataSerializer.java | 15 ++- .../java/com/fastsync/sync/SyncManager.java | 87 +++++++++++-- .../sync/dirty/ComponentDirtyMask.java | 12 +- .../chaos/ComponentStorageChaosTest.java | 48 +++++++ .../chaos/DatabaseComponentChaosTest.java | 26 ++++ .../PlayerDataSerializerComponentTest.java | 22 ++++ .../sync/SyncManagerComponentOverlayTest.java | 15 +++ .../fastsync/sync/SyncManagerRound15Test.java | 119 +++++++++++++++++- .../sync/dirty/ComponentDirtyMaskTest.java | 12 ++ 11 files changed, 348 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/fastsync/data/PlayerData.java b/src/main/java/com/fastsync/data/PlayerData.java index 68bb212..8515bc3 100644 --- a/src/main/java/com/fastsync/data/PlayerData.java +++ b/src/main/java/com/fastsync/data/PlayerData.java @@ -79,6 +79,10 @@ public class PlayerData { // Runtime-only marker: this instance contains only a dirty component // subset and must never be serialized as the authoritative full Blob. private boolean componentSubset; + // Runtime-only mask of components actually collected into a subset. This + // freezes the entity-thread config decision across the async persistence + // boundary, where a config reload may otherwise enable an uncollected field. + private long collectedComponentMask; public PlayerData() { this(true); @@ -209,6 +213,10 @@ public static PlayerData forComponentSubset() { public boolean isComponentSubset() { return componentSubset; } public void setComponentSubset(boolean componentSubset) { this.componentSubset = componentSubset; } + public void markComponentCollected(long storageMask) { collectedComponentMask |= storageMask; } + public boolean isComponentCollected(long storageMask) { + return (collectedComponentMask & storageMask) != 0L; + } // ==================== Inner Data Classes ==================== diff --git a/src/main/java/com/fastsync/database/DatabaseManager.java b/src/main/java/com/fastsync/database/DatabaseManager.java index d518056..c4f3a0e 100644 --- a/src/main/java/com/fastsync/database/DatabaseManager.java +++ b/src/main/java/com/fastsync/database/DatabaseManager.java @@ -1471,7 +1471,8 @@ public ComponentBatchResult upsertComponentsIfLockHeld( conn.setAutoCommit(oldAutoCommit); return ComponentBatchResult.rejected("stale collected version (expected: " + expectedVersion + ", actual: " + currentVersion + ")", - ComponentRejectReason.STALE_VERSION); + ComponentRejectReason.STALE_VERSION, currentVersion, + currentBitmap, generation); } // 3. Upsert component rows with current generation diff --git a/src/main/java/com/fastsync/serialization/PlayerDataSerializer.java b/src/main/java/com/fastsync/serialization/PlayerDataSerializer.java index 475149e..2dc71db 100644 --- a/src/main/java/com/fastsync/serialization/PlayerDataSerializer.java +++ b/src/main/java/com/fastsync/serialization/PlayerDataSerializer.java @@ -384,12 +384,15 @@ private static ItemStack[] fromItemStackList(ListTag list) { ItemStack[] items = new ItemStack[list.size()]; for (int i = 0; i < list.size(); i++) { Tag element = list.get(i); - if (element instanceof net.momirealms.sparrow.nbt.ByteArrayTag baTag) { - // ItemStackCompat.deserialize returns null for an empty payload - // (a real air slot) and throws ItemSerializationException on - // genuine corruption. Both behaviors are correct here. - items[i] = ItemStackCompat.deserialize(baTag.getAsByteArray()); - } + if (!(element instanceof net.momirealms.sparrow.nbt.ByteArrayTag baTag)) { + throw new ItemSerializationException("Invalid item tag at slot " + i + + ": expected ByteArrayTag, got " + + (element == null ? "null" : element.getClass().getSimpleName())); + } + // ItemStackCompat.deserialize returns null for an empty payload + // (a real air slot) and throws ItemSerializationException on + // genuine corruption. Both behaviors are correct here. + items[i] = ItemStackCompat.deserialize(baTag.getAsByteArray()); } return items; } diff --git a/src/main/java/com/fastsync/sync/SyncManager.java b/src/main/java/com/fastsync/sync/SyncManager.java index 8cd3744..f559e50 100644 --- a/src/main/java/com/fastsync/sync/SyncManager.java +++ b/src/main/java/com/fastsync/sync/SyncManager.java @@ -277,7 +277,7 @@ private static Attribute loadMaxHealthAttribute() { private volatile List aliveEntities; // isAlive() == true // Parsed snapshot trigger set (computed once at init/reload, O(1) lookup per save). - private volatile java.util.Set snapshotTriggerSet; + private volatile java.util.Set snapshotTriggerSet = java.util.Set.of(); // Sync strategies (initialized in initialize()) private com.fastsync.sync.strategy.PdcSyncStrategy pdcStrategy; @@ -651,6 +651,8 @@ private LoadResult loadPlayerDataInternal(UUID uuid) { databaseManager.loadPlayerDataRow(uuid); componentCursors.put(uuid, new ComponentCursor(loaded.componentGeneration(), loaded.componentBitmap())); + verifyComponentBaselineConsistency(uuid, loaded.hasData(), + loaded.componentBitmap(), loaded.componentGeneration()); long loadElapsedMs = (System.nanoTime() - startTime) / 1_000_000; latencyMonitor.recordLoad(loadElapsedMs); @@ -1661,12 +1663,16 @@ private PlayerData collectPlayerData(Player player, data.setArmor(snapshotItemContents(inventory.getArmorContents())); org.bukkit.inventory.ItemStack offhand = inventory.getItemInOffHand(); data.setOffhand(offhand != null && !offhand.getType().isAir() ? offhand.clone() : null); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.INVENTORY); } // Ender chest if (collects(requested, com.fastsync.sync.dirty.ComponentDirtyMask.Component.ENDER_CHEST) && config.isSyncEnderChest()) { data.setEnderChest(snapshotItemContents(player.getEnderChest().getStorageContents())); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.ENDER_CHEST); } // Vitals @@ -1698,6 +1704,8 @@ private PlayerData collectPlayerData(Player player, boolean dead = deathState != null || player.isDead() || currentHealth <= 0; data.setHealth(healthForSave(dead, currentHealth, effectiveMaxHealth)); data.setMaxHealth(baseMaxHealth); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.VITALS); } if (collects(requested, com.fastsync.sync.dirty.ComponentDirtyMask.Component.FOOD) && config.isSyncFood()) { @@ -1711,6 +1719,8 @@ private PlayerData collectPlayerData(Player player, data.setSaturation(player.getSaturation()); data.setExhaustion(player.getExhaustion()); } + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.FOOD); } // Experience @@ -1729,6 +1739,8 @@ private PlayerData collectPlayerData(Player player, data.setExpProgress(player.getExp()); data.setTotalExperience(player.getTotalExperience()); } + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.EXPERIENCE); } // Potion effects @@ -1741,16 +1753,22 @@ private PlayerData collectPlayerData(Player player, } } data.setPotionEffects(effects); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.POTION_EFFECTS); } // Extra if (collects(requested, com.fastsync.sync.dirty.ComponentDirtyMask.Component.GAME_MODE) && config.isSyncGameMode()) { data.setGameMode(player.getGameMode()); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.GAME_MODE); } if (collects(requested, com.fastsync.sync.dirty.ComponentDirtyMask.Component.FIRE_TICKS) && config.isSyncFireTicks()) { data.setFireTicks(deathState != null || player.isDead() ? 0 : player.getFireTicks()); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.FIRE_TICKS); } if (collects(requested, com.fastsync.sync.dirty.ComponentDirtyMask.Component.AIR) && config.isSyncAir()) { @@ -1758,6 +1776,8 @@ private PlayerData collectPlayerData(Player player, data.setMaximumAir(maximumAir); data.setRemainingAir( deathState != null || player.isDead() ? maximumAir : player.getRemainingAir()); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.AIR); } // Flight status @@ -1783,24 +1803,32 @@ private PlayerData collectPlayerData(Player player, } data.setFlying(flying); data.setAllowFlight(allowFlight); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.FLIGHT); } // Advancements (using Bukkit API - iterates all advancement criteria) if (collects(requested, com.fastsync.sync.dirty.ComponentDirtyMask.Component.ADVANCEMENTS) && config.isSyncAdvancements()) { collectAdvancements(player, data); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.ADVANCEMENTS); } // Statistics (basic UNTYPED stats always synced; typed stats via strategy) if (collects(requested, com.fastsync.sync.dirty.ComponentDirtyMask.Component.STATISTICS) && config.isSyncStatistics()) { collectStatistics(player, data); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.STATISTICS); } // Attributes if (collects(requested, com.fastsync.sync.dirty.ComponentDirtyMask.Component.ATTRIBUTES) && config.isSyncAttributes()) { collectAttributes(player, data); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.ATTRIBUTES); } // Persistent Data Container (via strategy) @@ -1816,6 +1844,8 @@ private PlayerData collectPlayerData(Player player, } else { data.setPersistentDataContainer(new HashMap<>()); } + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.PDC); } // Location (optional) @@ -1829,6 +1859,8 @@ private PlayerData collectPlayerData(Player player, data.setZ(loc.getZ()); data.setYaw(loc.getYaw()); data.setPitch(loc.getPitch()); + markComponentCollected(data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.LOCATION); } data.setTimestamp(System.currentTimeMillis()); @@ -1842,6 +1874,13 @@ private static boolean collects( return requested == null || requested.contains(component); } + private static void markComponentCollected(PlayerData data, + com.fastsync.sync.dirty.ComponentDirtyMask.Component component) { + if (data.isComponentSubset()) { + data.markComponentCollected(component.storageMask()); + } + } + /** * Detach Paper's CraftItemStack mirrors while still on the entity thread. * CraftInventory#getStorageContents returns a new array, but Paper 1.21.11 @@ -1866,6 +1905,7 @@ private static org.bukkit.inventory.ItemStack[] snapshotItemContents( private boolean canCollectComponentsOnly(UUID uuid, SaveKind kind, com.fastsync.sync.dirty.ComponentDirtyMask.DirtySnapshot snapshot) { if (!config.isComponentStorageEnabled() || dirtyMask == null || kind.releaseLock + || (snapshotManager != null && shouldCreateSnapshot(kind.causeName)) || snapshot == null || snapshot.isEmpty() || !playersWithBaseline.contains(uuid) || !componentCursors.containsKey(uuid)) { return false; @@ -3424,8 +3464,15 @@ private ComponentSaveOutcome persistComponentsOnly(UUID uuid, PlayerData data, S for (com.fastsync.sync.dirty.ComponentDirtyMask.Component c : dirty) { String name = c.name(); - // Skip components that are disabled in config - if (!isComponentSyncEnabled(c)) continue; + // A selective carrier freezes the entity-thread collection + // decision. Re-reading config here is unsafe: a reload between + // collect and async persistence could enable an uncollected + // primitive field and write its Java default as authoritative. + if (data.isComponentSubset()) { + if (!data.isComponentCollected(c.storageMask())) continue; + } else if (!isComponentSyncEnabled(c)) { + continue; + } byte[] serialized = PlayerDataSerializer.serializeComponent(name, data); if (serialized == null) continue; // component has no data @@ -3517,8 +3564,16 @@ private ComponentSaveOutcome persistComponentsOnly(UUID uuid, PlayerData data, S // Our collected snapshot is behind the DB version. // Skip THIS online save — do NOT fall back to a full // Blob save with the stale snapshot (it would clobber - // the newer state). The next periodic cycle re-collects - // against the current version. + // the newer state). The rejection carries metadata read + // only after lock/fencing/session validation, so advance + // the session-local version/cursor before the next cycle. + // Without this refresh every later component CAS repeats + // the same stale expectedVersion forever. + if (batchResult.newVersion() > data.getVersion()) { + playerVersions.put(uuid, batchResult.newVersion()); + componentCursors.put(uuid, new ComponentCursor( + batchResult.generation(), batchResult.componentBitmap())); + } logger.warning("[Component] Save rejected for " + uuid + " (STALE_VERSION): " + batchResult.errorMessage() + " — skipping this online save; next cycle re-collects."); @@ -3574,7 +3629,7 @@ private ComponentSaveOutcome persistComponentsOnly(UUID uuid, PlayerData data, S // concurrent markDirty bumped an epoch during the DB write, that // component stays dirty and the next periodic save will re-serialize // and re-write it with the latest state. - dirtyMask.clearDirty(uuid, dirtySnapshot); + dirtyMask.clearDirty(uuid, dirtySnapshot, dirtyBits); if (config.isDebug()) { logger.info("Component save " + kind + " for " + uuid + ": " @@ -3759,6 +3814,7 @@ private SaveResult persistCollectedData(UUID uuid, PlayerData data, SaveKind kin if (config.isComponentStorageEnabled() && dirtyMask != null && !kind.releaseLock + && (snapshotManager == null || !shouldCreateSnapshot(data.getSaveCause())) && dirtyMask.isAnyDirty(uuid)) { // Pass the caller-provided snapshot (taken before collectPlayerData) // so persistComponentsOnly's clearDirty after the DB write protects @@ -4020,8 +4076,13 @@ private SaveResult persistCollectedData(UUID uuid, PlayerData data, SaveKind kin // From this point on, component-only saves are safe for this player // for the remainder of the session. playersWithBaseline.add(uuid); - componentCursors.compute(uuid, (ignored, current) -> - new ComponentCursor(current == null ? 1L : current.generation() + 1L, 0L)); + // Advance only a cursor that was loaded authoritatively. Inventing + // generation=1 when the cursor is absent is wrong after a hot reload + // if the DB row is already at generation N. Leaving it absent makes + // the next component save use the compatibility transaction, which + // locks the row, reads the real generation, and seeds a correct cursor. + componentCursors.computeIfPresent(uuid, (ignored, current) -> + new ComponentCursor(current.generation() + 1L, 0L)); // Clear dirty flags using the pre-save snapshot's epoch. // @@ -4461,6 +4522,16 @@ static void verifyComponentOverlayCompleteness( } } + static void verifyComponentBaselineConsistency( + UUID uuid, boolean hasBaselineData, long componentBitmap, + long generation) throws IOException { + if (!hasBaselineData && componentBitmap != 0L) { + throw new IOException("component_bitmap is non-zero without a full-Blob baseline for " + + uuid + " (gen=" + generation + ", bitmap=0x" + + Long.toHexString(componentBitmap) + ")"); + } + } + static java.util.Set componentNamesForBitmap(long bitmap) throws IOException { java.util.Set names = new java.util.HashSet<>(); long knownMask = 0L; diff --git a/src/main/java/com/fastsync/sync/dirty/ComponentDirtyMask.java b/src/main/java/com/fastsync/sync/dirty/ComponentDirtyMask.java index 7c083be..d8dc3d8 100644 --- a/src/main/java/com/fastsync/sync/dirty/ComponentDirtyMask.java +++ b/src/main/java/com/fastsync/sync/dirty/ComponentDirtyMask.java @@ -166,7 +166,13 @@ public DirtySnapshot snapshotDirty(UUID uuid) { */ public void clearDirty(UUID uuid, DirtySnapshot snapshot) { PlayerDirtyState state = masks.get(uuid); - if (state != null) state.clearIfEpochMatches(snapshot); + if (state != null) state.clearIfEpochMatches(snapshot, ~0L); + } + + /** Clear only successfully persisted snapshot components. */ + public void clearDirty(UUID uuid, DirtySnapshot snapshot, long persistedBits) { + PlayerDirtyState state = masks.get(uuid); + if (state != null) state.clearIfEpochMatches(snapshot, persistedBits); } /** @@ -286,10 +292,10 @@ DirtySnapshot snapshotWithEpochs() { return bits == 0 ? DirtySnapshot.EMPTY : new DirtySnapshot(bits, snapshotStates); } - void clearIfEpochMatches(DirtySnapshot snapshot) { + void clearIfEpochMatches(DirtySnapshot snapshot, long persistedBits) { if (snapshot == null || snapshot.isEmpty()) return; for (Component component : Component.values()) { - if (!snapshot.contains(component)) continue; + if (!snapshot.contains(component) || (persistedBits & bit(component)) == 0L) continue; int index = component.ordinal(); long expected = snapshot.states[index]; states.compareAndSet(index, expected, expected & ~DIRTY_FLAG); diff --git a/src/test/java/com/fastsync/chaos/ComponentStorageChaosTest.java b/src/test/java/com/fastsync/chaos/ComponentStorageChaosTest.java index d8f2959..0c790f8 100644 --- a/src/test/java/com/fastsync/chaos/ComponentStorageChaosTest.java +++ b/src/test/java/com/fastsync/chaos/ComponentStorageChaosTest.java @@ -1,5 +1,6 @@ package com.fastsync.chaos; +import com.fastsync.sync.dirty.ComponentDirtyMask; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -85,6 +86,53 @@ void testChaosInvariantsHold() { } } + @Test + void dirtyAcknowledgementChaosNeverClearsUncommittedOrConcurrentChanges() { + ComponentDirtyMask mask = new ComponentDirtyMask(5); + UUID uuid = UUID.fromString("00000000-0000-0000-0000-000000000999"); + ComponentDirtyMask.Component[] components = ComponentDirtyMask.Component.values(); + Random interleaving = new Random(0xFA57C0DEL); + + for (int iteration = 0; iteration < ITERATIONS; iteration++) { + mask.clearAll(uuid); + long initialBits = 0L; + for (ComponentDirtyMask.Component component : components) { + if (interleaving.nextBoolean()) { + mask.markDirty(uuid, component); + initialBits |= component.storageMask(); + } + } + if (initialBits == 0L) { + ComponentDirtyMask.Component component = components[interleaving.nextInt(components.length)]; + mask.markDirty(uuid, component); + initialBits |= component.storageMask(); + } + + ComponentDirtyMask.DirtySnapshot snapshot = mask.snapshotDirty(uuid); + long persistedBits = 0L; + long concurrentBits = 0L; + for (ComponentDirtyMask.Component component : components) { + if ((initialBits & component.storageMask()) != 0L && interleaving.nextBoolean()) { + persistedBits |= component.storageMask(); + } + if (interleaving.nextInt(5) == 0) { + mask.markDirty(uuid, component); + concurrentBits |= component.storageMask(); + } + } + + mask.clearDirty(uuid, snapshot, persistedBits); + + for (ComponentDirtyMask.Component component : components) { + long bit = component.storageMask(); + boolean mustRemainDirty = (concurrentBits & bit) != 0L + || ((initialBits & bit) != 0L && (persistedBits & bit) == 0L); + assertEquals(mustRemainDirty, mask.getDirty(uuid).contains(component), + "iteration=" + iteration + ", component=" + component); + } + } + } + // ==================== Simulated Operations ==================== private void simulateAcquireLock(UUID uuid, String serverName) { diff --git a/src/test/java/com/fastsync/chaos/DatabaseComponentChaosTest.java b/src/test/java/com/fastsync/chaos/DatabaseComponentChaosTest.java index b567308..174f680 100644 --- a/src/test/java/com/fastsync/chaos/DatabaseComponentChaosTest.java +++ b/src/test/java/com/fastsync/chaos/DatabaseComponentChaosTest.java @@ -160,6 +160,32 @@ void testCursorComponentHotPathCommitsAndRejectsStaleGeneration() throws SQLExce "Rejected cursor CAS must roll back the metadata version bump"); } + @Test + void staleVersionRejectionReturnsAuthoritativeRecoveryCursor() throws SQLException { + UUID uuid = UUID.randomUUID(); + String server = "server-A"; + String session = "stale-version-session"; + var lock = databaseManager.acquireLock(uuid, server, session); + assertTrue(lock.acquired()); + + Map components = Map.of("FOOD", new byte[]{1, 2, 3}); + Map checksums = Map.of("FOOD", 123L); + long foodBit = 1L << 3; + + var saved = databaseManager.upsertComponentsIfLockHeld( + uuid, components, checksums, server, lock.fencingToken(), session, 0L, foodBit); + assertTrue(saved.success()); + + var stale = databaseManager.upsertComponentsIfLockHeld( + uuid, components, checksums, server, lock.fencingToken(), session, 0L, foodBit); + + assertFalse(stale.success()); + assertEquals(DatabaseManager.ComponentRejectReason.STALE_VERSION, stale.reason()); + assertEquals(saved.newVersion(), stale.newVersion()); + assertEquals(saved.componentBitmap(), stale.componentBitmap()); + assertEquals(saved.generation(), stale.generation()); + } + @Test void testComponentSaveWithStaleFencingRejected() throws SQLException { UUID uuid = UUID.randomUUID(); diff --git a/src/test/java/com/fastsync/serialization/PlayerDataSerializerComponentTest.java b/src/test/java/com/fastsync/serialization/PlayerDataSerializerComponentTest.java index 411fb4e..9610ea6 100644 --- a/src/test/java/com/fastsync/serialization/PlayerDataSerializerComponentTest.java +++ b/src/test/java/com/fastsync/serialization/PlayerDataSerializerComponentTest.java @@ -300,6 +300,28 @@ void testDeserializeMissingPresenceMarkerFailsClosed() throws IOException { () -> PlayerDataSerializer.deserializeComponent("VITALS", bytes, new PlayerData())); } + @Test + void inventoryComponentRejectsNonByteArrayItemTags() throws IOException { + net.momirealms.sparrow.nbt.CompoundTag wrapper = + net.momirealms.sparrow.nbt.NBT.createCompound(); + net.momirealms.sparrow.nbt.CompoundTag inventory = + net.momirealms.sparrow.nbt.NBT.createCompound(); + net.momirealms.sparrow.nbt.ListTag invalidItems = + net.momirealms.sparrow.nbt.NBT.createList(); + invalidItems.add(net.momirealms.sparrow.nbt.NBT.createString("not-an-item-payload")); + inventory.put("inventory", invalidItems); + inventory.putBoolean("offhandPresent", false); + inventory.putBoolean("_present", true); + wrapper.put("INVENTORY", inventory); + + byte[] bytes = net.momirealms.sparrow.nbt.NBT.toBytes(wrapper); + + ItemSerializationException error = assertThrows(ItemSerializationException.class, + () -> PlayerDataSerializer.deserializeComponent("INVENTORY", bytes, new PlayerData())); + assertTrue(error.getMessage().contains("slot 0")); + assertTrue(error.getMessage().contains("ByteArrayTag")); + } + @Test void testComponentIsolation() throws IOException { PlayerData original = new PlayerData(); diff --git a/src/test/java/com/fastsync/sync/SyncManagerComponentOverlayTest.java b/src/test/java/com/fastsync/sync/SyncManagerComponentOverlayTest.java index cd60f30..3e87be0 100644 --- a/src/test/java/com/fastsync/sync/SyncManagerComponentOverlayTest.java +++ b/src/test/java/com/fastsync/sync/SyncManagerComponentOverlayTest.java @@ -57,4 +57,19 @@ void verifyComponentOverlayCompletenessRejectsMissingRows() { assertTrue(ex.getMessage().contains("gen=9"), "Error must include the component generation for diagnosis"); } + + @Test + void nonZeroBitmapWithoutBaselineFailsClosed() { + UUID uuid = UUID.randomUUID(); + + IOException error = assertThrows(IOException.class, + () -> SyncManager.verifyComponentBaselineConsistency(uuid, false, 0x4L, 9L)); + + assertTrue(error.getMessage().contains(uuid.toString())); + assertTrue(error.getMessage().contains("gen=9")); + assertDoesNotThrow( + () -> SyncManager.verifyComponentBaselineConsistency(uuid, true, 0x4L, 9L)); + assertDoesNotThrow( + () -> SyncManager.verifyComponentBaselineConsistency(uuid, false, 0L, 9L)); + } } diff --git a/src/test/java/com/fastsync/sync/SyncManagerRound15Test.java b/src/test/java/com/fastsync/sync/SyncManagerRound15Test.java index 179ae01..04025a2 100644 --- a/src/test/java/com/fastsync/sync/SyncManagerRound15Test.java +++ b/src/test/java/com/fastsync/sync/SyncManagerRound15Test.java @@ -224,6 +224,12 @@ void finalSaveRetriesSameSessionVersionConflictThreeTimes() throws Exception { // Verify getLockState was called after each failed attempt (2 times). verify(databaseManager, times(2)).getLockState(eq(uuid)); + + // No authoritative component cursor was loaded in this test. A full + // save must not invent generation=1; real hot-reload sessions may be at + // any generation, so the compatibility DB read must seed it later. + ConcurrentHashMap componentCursors = getField("componentCursors"); + assertFalse(componentCursors.containsKey(uuid)); } /** @@ -346,6 +352,8 @@ void selectiveComponentCollectDoesNotReadUnrelatedPaperState() throws Exception PlayerData data = (PlayerData) method.invoke(syncManager, player, snapshot); assertTrue(data.isComponentSubset()); + assertTrue(data.isComponentCollected( + com.fastsync.sync.dirty.ComponentDirtyMask.Component.FOOD.storageMask())); assertEquals(17, data.getFoodLevel()); assertEquals(4.5F, data.getSaturation()); assertNull(data.getInventory()); @@ -373,6 +381,99 @@ void partialComponentCarrierCanNeverFallThroughToFullBlob() throws Exception { any(), any(), anyLong(), anyLong(), anyLong(), any(), any()); } + @Test + void configReloadCannotPersistUncollectedSubsetComponents() throws Exception { + UUID uuid = UUID.randomUUID(); + long foodBit = com.fastsync.sync.dirty.ComponentDirtyMask.Component.FOOD.storageMask(); + + PlayerData partial = PlayerData.forComponentSubset(); + partial.setVersion(3L); + partial.setFencingToken(5L); + partial.setFoodLevel(18); + partial.setSaturation(4.0F); + partial.markComponentCollected(foodBit); + + when(config.isComponentStorageEnabled()).thenReturn(true); + when(config.getComponentBatchSize()).thenReturn(10); + // Simulate a reload after collection: FOOD was collected under the old + // config, while VITALS is enabled only now and is absent from `partial`. + when(config.isSyncFood()).thenReturn(false); + when(config.isSyncHealth()).thenReturn(true); + + com.fastsync.sync.dirty.ComponentDirtyMask dirtyMask = + mock(com.fastsync.sync.dirty.ComponentDirtyMask.class); + when(dirtyMask.isAnyDirty(uuid)).thenReturn(true); + com.fastsync.sync.dirty.ComponentDirtyMask.DirtySnapshot snapshot = + mock(com.fastsync.sync.dirty.ComponentDirtyMask.DirtySnapshot.class); + when(snapshot.isEmpty()).thenReturn(false); + when(snapshot.components()).thenReturn(java.util.Set.of( + com.fastsync.sync.dirty.ComponentDirtyMask.Component.FOOD, + com.fastsync.sync.dirty.ComponentDirtyMask.Component.VITALS)); + injectField("dirtyMask", dirtyMask); + java.util.Set baseline = getField("playersWithBaseline"); + baseline.add(uuid); + ConcurrentHashMap sessions = getField("playerLockSessions"); + sessions.put(uuid, "reload-session"); + + when(databaseManager.upsertComponentsIfLockHeld( + eq(uuid), anyMap(), anyMap(), eq("test-server"), eq(5L), + eq("reload-session"), eq(3L), eq(foodBit))) + .thenReturn(DatabaseManager.ComponentBatchResult.success(3L, 4L, foodBit, 2L)); + + SyncManager.SaveResult result = invokePersistCollectedData( + uuid, partial, SyncManager.SaveKind.PERIODIC, snapshot); + + assertTrue(result.success()); + verify(databaseManager).upsertComponentsIfLockHeld( + eq(uuid), argThat(map -> map.keySet().equals(java.util.Set.of("FOOD"))), + argThat(map -> map.keySet().equals(java.util.Set.of("FOOD"))), + eq("test-server"), eq(5L), eq("reload-session"), eq(3L), eq(foodBit)); + verify(dirtyMask).clearDirty(uuid, snapshot, foodBit); + } + + @Test + void snapshotTriggerForcesFullBlobInsteadOfComponentFastPath() throws Exception { + UUID uuid = UUID.randomUUID(); + PlayerData data = new PlayerData(); + data.setVersion(3L); + data.setFencingToken(5L); + + when(config.isComponentStorageEnabled()).thenReturn(true); + when(config.getMaxSnapshots()).thenReturn(16); + com.fastsync.sync.dirty.ComponentDirtyMask dirtyMask = + mock(com.fastsync.sync.dirty.ComponentDirtyMask.class); + when(dirtyMask.isAnyDirty(uuid)).thenReturn(true); + injectField("dirtyMask", dirtyMask); + injectField("snapshotTriggerSet", java.util.Set.of("death")); + + ConcurrentHashMap sessions = getField("playerLockSessions"); + sessions.put(uuid, "snapshot-session"); + when(databaseManager.saveDataKeepLockClearComponents( + eq(uuid), any(byte[].class), anyLong(), eq(3L), eq(5L), + eq("test-server"), eq("snapshot-session"))) + .thenReturn(true); + + SnapshotManager snapshots = mock(SnapshotManager.class); + when(snapshots.createSnapshot(eq(uuid), any(byte[].class), eq("death"))) + .thenReturn(java.util.concurrent.CompletableFuture.completedFuture(1L)); + when(snapshots.pruneSnapshots(uuid, 16)) + .thenReturn(java.util.concurrent.CompletableFuture.completedFuture(null)); + injectField("snapshotManager", snapshots); + + SyncManager.SaveResult result = invokePersistCollectedData( + uuid, data, SyncManager.SaveKind.DEATH, + com.fastsync.sync.dirty.ComponentDirtyMask.DirtySnapshot.EMPTY); + + assertTrue(result.success()); + verify(databaseManager).saveDataKeepLockClearComponents( + eq(uuid), any(byte[].class), anyLong(), eq(3L), eq(5L), + eq("test-server"), eq("snapshot-session")); + verify(databaseManager, never()).upsertComponentsIfLockHeld( + any(UUID.class), anyMap(), anyMap(), anyString(), anyLong(), + anyString(), anyLong(), anyLong()); + verify(snapshots).createSnapshot(eq(uuid), any(byte[].class), eq("death")); + } + /** * Round 15 test: componentStaleVersionDoesNotFallbackFullBlob. * @@ -418,7 +519,8 @@ void componentStaleVersionDoesNotFallbackFullBlob() throws Exception { // STALE_VERSION rejection. DatabaseManager.ComponentBatchResult rejected = DatabaseManager.ComponentBatchResult.rejected( - "stale collected version", DatabaseManager.ComponentRejectReason.STALE_VERSION); + "stale collected version", DatabaseManager.ComponentRejectReason.STALE_VERSION, + 4L, 0x5L, 7L); when(databaseManager.upsertComponentsIfLockHeld( any(UUID.class), anyMap(), anyMap(), any(String.class), anyLong(), any(String.class), anyLong(), anyLong())) @@ -439,6 +541,21 @@ void componentStaleVersionDoesNotFallbackFullBlob() throws Exception { assertEquals(SyncManager.SaveFailureReason.VERSION_CONFLICT, result.failureReason(), "Stale version must map to VERSION_CONFLICT, not fall back"); + // The DB rejection is authoritative only after the same lock/fencing/ + // session was verified. Refresh both local cursors so the next cycle + // does not repeat expectedVersion=3 forever. + ConcurrentHashMap playerVersions = getField("playerVersions"); + assertEquals(4L, playerVersions.get(uuid)); + ConcurrentHashMap componentCursors = getField("componentCursors"); + Object cursor = componentCursors.get(uuid); + assertNotNull(cursor); + Method generation = cursor.getClass().getDeclaredMethod("generation"); + Method bitmap = cursor.getClass().getDeclaredMethod("bitmap"); + generation.setAccessible(true); + bitmap.setAccessible(true); + assertEquals(7L, generation.invoke(cursor)); + assertEquals(0x5L, bitmap.invoke(cursor)); + // Full Blob save must NOT be called. verify(databaseManager, never()) .saveDataKeepLockClearComponents( diff --git a/src/test/java/com/fastsync/sync/dirty/ComponentDirtyMaskTest.java b/src/test/java/com/fastsync/sync/dirty/ComponentDirtyMaskTest.java index a23cc1b..8a3a559 100644 --- a/src/test/java/com/fastsync/sync/dirty/ComponentDirtyMaskTest.java +++ b/src/test/java/com/fastsync/sync/dirty/ComponentDirtyMaskTest.java @@ -293,6 +293,18 @@ void testClearDirtyClearsMatchingComponentsOnly() { assertTrue(dirty.contains(ComponentDirtyMask.Component.INVENTORY)); } + @Test + void persistedBitFilterDoesNotClearUnwrittenSnapshotComponents() { + mask.markDirty(player1, ComponentDirtyMask.Component.VITALS); + mask.markDirty(player1, ComponentDirtyMask.Component.FOOD); + ComponentDirtyMask.DirtySnapshot snapshot = mask.snapshotDirty(player1); + + mask.clearDirty(player1, snapshot, + ComponentDirtyMask.Component.VITALS.storageMask()); + + assertEquals(Set.of(ComponentDirtyMask.Component.FOOD), mask.getDirty(player1)); + } + /** * Verifies that snapshotDirty on a player with no mask returns EMPTY * (not null) and clearDirty(EMPTY) is a no-op.