R3944Realms/PlayerSync
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix last 2 audit issues: syncNotCompleted race + SaveToFile off-thread

Browse Source 
BUG 1 - syncNotCompletedPlayer race condition:
  syncNotCompletedPlayer.add() was inside the background thread body.
  A player disconnecting instantly before the thread starts bypasses
  the "sync not completed" guard in onPlayerLogout, causing store()
  to read invalid entity state.
  FIX: add() moved to onPlayerJoin BEFORE executorService.submit().

BUG 2 - doPlayerSaveToFile off main thread:
  onPlayerSaveToFile wrapped doPlayerSaveToFile in executorService,
  but SaveToFile already fires on the main thread. store() reads
  player inventory/armor/effects from a background thread = corruption.
  FIX: Call doPlayerSaveToFile directly (no executor). Same fix as
  auto-save and logout paths.

Vyrriox
This commit is contained in:
laforetbrut 2026-03-26 19:17:16 +01:00
parent 6bb8aeba39
commit 50c77f7bb8
1 changed files with 16 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
src/main/java/vip/fubuki/playersync/sync/VanillaSync.java
View File

@ -258,7 +258,7 @@ public class VanillaSync {
lock.lock(); lock.lock();
try { try {
PlayerSync.LOGGER.info("Starting synchronization for player {}", player_uuid); PlayerSync.LOGGER.info("Starting synchronization for player {}", player_uuid);
syncNotCompletedPlayer.add(player_uuid); // syncNotCompletedPlayer.add() already done in onPlayerJoin before submit
// === PHASE 1: DB reads on background thread (thread-safe) === // === PHASE 1: DB reads on background thread (thread-safe) ===
@ -494,11 +494,18 @@ public class VanillaSync {
@SubscribeEvent @SubscribeEvent
public static void onPlayerJoin(PlayerEvent.PlayerLoggedInEvent event) { public static void onPlayerJoin(PlayerEvent.PlayerLoggedInEvent event) {
// FIX: Mark sync as pending BEFORE submitting to thread pool.
// Without this, a player who disconnects instantly can trigger onPlayerLogout
// before the background thread starts, bypassing the syncNotCompleted guard
// and saving invalid entity state.
String puuid = ((ServerPlayer) event.getEntity()).getUUID().toString();
syncNotCompletedPlayer.add(puuid);
executorService.submit(() -> { executorService.submit(() -> {
try { try {
doPlayerJoin(event); doPlayerJoin(event);
} catch (Exception e) { } catch (Exception e) {
e.printStackTrace(); e.printStackTrace();
syncNotCompletedPlayer.remove(puuid);
} }
}); });
} }
@ -685,15 +692,16 @@ public class VanillaSync {
store(event.getEntity(), false); store(event.getEntity(), false);
} }
// FIX: SaveToFile already fires on the main thread. Running store() off-thread via
// executorService read player entity state (inventory, armor, effects) from a background
// thread, causing duplication/corruption. Run directly on the main thread.
@SubscribeEvent @SubscribeEvent
public static void onPlayerSaveToFile(PlayerEvent.SaveToFile event) { public static void onPlayerSaveToFile(PlayerEvent.SaveToFile event) {
executorService.submit(() -> { try {
try { doPlayerSaveToFile(event);
doPlayerSaveToFile(event); } catch (Exception e) {
} catch (Exception e) { PlayerSync.LOGGER.error("Error during player save-to-file", e);
e.printStackTrace(); }
}
});
} }
@SubscribeEvent @SubscribeEvent