Add timeout to AsyncPlayerDataSaving (#275)

* Add timeout to AsyncPlayerDataSaving * dump thread if failed to save playerdata * PlayerDataStorage#lockFor break loop after cancel
2025-12-21 16:09:19 +00:00 · 2025-03-31 23:34:44 +08:00
parent 8e11af1228
commit f1a31d88c8
3 changed files with 102 additions and 81 deletions
--- a/leaf-server/minecraft-patches/features/0086-Nitori-Async-playerdata-saving.patch
+++ b/leaf-server/minecraft-patches/features/0086-Nitori-Async-playerdata-saving.patch
@@ -1,7 +1,7 @@
 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: Dreeam <61569423+Dreeam-qwq@users.noreply.github.com>
 Date: Fri, 23 Aug 2024 22:04:20 -0400
-Subject: [PATCH] Nitori: Async playerdata Save
+Subject: [PATCH] Nitori: Async playerdata saving

 Original license: GPL v3
 Original project: https://github.com/Gensokyo-Reimagined/Nitori
@@ -24,7 +24,7 @@ index 5a0d30b8ff5e377224de67c9f464bd1c694a4397..1cb60107d95296fc9e2c106d70838c05
 
     public String getLocalIp() {
 diff --git a/net/minecraft/world/level/storage/LevelStorageSource.java b/net/minecraft/world/level/storage/LevelStorageSource.java
-index de43e54698125ce9f319d4889dd49f7029fe95e0..451cb8987906f0a9e753a870aa629124daba9c73 100644
+index de43e54698125ce9f319d4889dd49f7029fe95e0..742bd4b60321adc9e63c3de910ea95f4990b618d 100644
 --- a/net/minecraft/world/level/storage/LevelStorageSource.java
 +++ b/net/minecraft/world/level/storage/LevelStorageSource.java
@@ -520,15 +520,26 @@ public class LevelStorageSource {
@@ -45,7 +45,7 @@ index de43e54698125ce9f319d4889dd49f7029fe95e0..451cb8987906f0a9e753a870aa629124
 -                LevelStorageSource.LOGGER.error("Failed to save level {}", path, var6);
 +                LevelStorageSource.LOGGER.error("Failed to encode level {}", path, var6);
             }
-+            org.dreeam.leaf.async.AsyncPlayerDataSaving.save(() -> {
+            org.dreeam.leaf.async.AsyncPlayerDataSaving.submit(() -> {
 +                try {
 +                    Path path1 = Files.createTempFile(path, "level", ".dat");
 +                    org.apache.commons.io.FileUtils.writeByteArrayToFile(path1.toFile(), nbtBytes.array, 0, nbtBytes.length, false);
@@ -61,25 +61,25 @@ index de43e54698125ce9f319d4889dd49f7029fe95e0..451cb8987906f0a9e753a870aa629124
 
         public Optional<Path> getIconFile() {
 diff --git a/net/minecraft/world/level/storage/PlayerDataStorage.java b/net/minecraft/world/level/storage/PlayerDataStorage.java
-index c44110b123ba5912af18faf0065e9ded780da9b7..541c440d3211c6625974ac1e0055d62655256846 100644
+index c44110b123ba5912af18faf0065e9ded780da9b7..bf5a697c602427768a164a8ab8af1074688d81fd 100644
 --- a/net/minecraft/world/level/storage/PlayerDataStorage.java
 +++ b/net/minecraft/world/level/storage/PlayerDataStorage.java
@@ -25,6 +25,7 @@ public class PlayerDataStorage {
     private final File playerDir;
     protected final DataFixer fixerUpper;
     private static final DateTimeFormatter FORMATTER = FileNameDateFormatter.create();
-+    public final it.unimi.dsi.fastutil.objects.ObjectOpenHashSet<java.util.UUID> savingQueue = new it.unimi.dsi.fastutil.objects.ObjectOpenHashSet<>(); // Leaf - Async playerdata saving
+    private final java.util.Map<java.util.UUID, java.util.concurrent.Future<?>> savingLocks = new it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap<>(); // Leaf - Async playerdata saving
 
     public PlayerDataStorage(LevelStorageSource.LevelStorageAccess levelStorageAccess, DataFixer fixerUpper) {
         this.fixerUpper = fixerUpper;
-@@ -34,17 +35,43 @@ public class PlayerDataStorage {
+@@ -34,19 +35,82 @@ public class PlayerDataStorage {
 
     public void save(Player player) {
         if (org.spigotmc.SpigotConfig.disablePlayerDataSaving) return; // Spigot
 +        // Leaf start - Async playerdata saving
-+        var nbtBytes = new it.unimi.dsi.fastutil.io.FastByteArrayOutputStream(65536);
+        CompoundTag compoundTag;
         try {
-             CompoundTag compoundTag = player.saveWithoutId(new CompoundTag());
+-            CompoundTag compoundTag = player.saveWithoutId(new CompoundTag());
 -            Path path = this.playerDir.toPath();
 -            Path path1 = Files.createTempFile(path, player.getStringUUID() + "-", ".dat");
 -            NbtIo.writeCompressed(compoundTag, path1);
@@ -88,43 +88,83 @@ index c44110b123ba5912af18faf0065e9ded780da9b7..541c440d3211c6625974ac1e0055d626
 -            Util.safeReplaceFile(path2, path1, path3);
 -        } catch (Exception var7) {
 -            LOGGER.warn("Failed to save player data for {}", player.getScoreboardName(), var7); // Paper - Print exception
-+            NbtIo.writeCompressed(compoundTag, nbtBytes);
+            compoundTag = player.saveWithoutId(new CompoundTag());
 +        } catch (Exception exception) {
 +            LOGGER.warn("Failed to encode player data for {}", player.getScoreboardName(), exception);
+            return;
         }
-+        String playerName = player.getScoreboardName();
-+        String stringUuid = player.getStringUUID();
-+        java.util.UUID playerUuid = player.getUUID();
-+        synchronized (PlayerDataStorage.this) {
-+            while (savingQueue.contains(playerUuid)) {
-+                try {
-+                    Thread.sleep(1L);
-+                } catch (InterruptedException ignored) {
-+                }
-+            }
-+            savingQueue.add(playerUuid);
-+        }
-+        org.dreeam.leaf.async.AsyncPlayerDataSaving.save(() -> {
-+            try {
-+                Path path = this.playerDir.toPath();
-+                Path path1 = Files.createTempFile(path, stringUuid + "-", ".dat");
-+                org.apache.commons.io.FileUtils.writeByteArrayToFile(path1.toFile(), nbtBytes.array, 0, nbtBytes.length, false);
-+                Path path2 = path.resolve(stringUuid + ".dat");
-+                Path path3 = path.resolve(stringUuid + ".dat_old");
-+                Util.safeReplaceFile(path2, path1, path3);
-+            } catch (Exception var7) {
-+                LOGGER.warn("Failed to save player data for {}", playerName, var7); // Paper - Print exception
-+            } finally {
-+                synchronized (PlayerDataStorage.this) {
-+                    savingQueue.remove(playerUuid);
-+                }
-+            }
-+        });
+        save(player.getScoreboardName(), player.getUUID(), player.getStringUUID(), compoundTag);
 +        // Leaf end - Async playerdata saving
     }
 
+    // Leaf start - Async playerdata saving
+    public void save(String playerName, java.util.UUID uniqueId, String stringId, CompoundTag compoundTag) {
+        var nbtBytes = new it.unimi.dsi.fastutil.io.FastByteArrayOutputStream(65536);
+        try {
+            NbtIo.writeCompressed(compoundTag, nbtBytes);
+        } catch (Exception exception) {
+            LOGGER.warn("Failed to encode player data for {}", stringId, exception);
+        }
+        lockFor(uniqueId, playerName);
+        synchronized (PlayerDataStorage.this) {
+            org.dreeam.leaf.async.AsyncPlayerDataSaving.submit(() -> {
+                try {
+                    Path path = this.playerDir.toPath();
+                    Path path1 = Files.createTempFile(path, stringId + "-", ".dat");
+                    org.apache.commons.io.FileUtils.writeByteArrayToFile(path1.toFile(), nbtBytes.array, 0, nbtBytes.length, false);
+                    Path path2 = path.resolve(stringId + ".dat");
+                    Path path3 = path.resolve(stringId + ".dat_old");
+                    Util.safeReplaceFile(path2, path1, path3);
+                } catch (Exception var7) {
+                    LOGGER.warn("Failed to save player data for {}", playerName, var7);
+                } finally {
+                    synchronized (PlayerDataStorage.this) {
+                        savingLocks.remove(uniqueId);
+                    }
+                }
+            }).ifPresent(future -> savingLocks.put(uniqueId, future));
+        }
+    }
+
+    private void lockFor(java.util.UUID uniqueId, String playerName) {
+        java.util.concurrent.Future<?> fut;
+        synchronized (this) {
+            fut = savingLocks.get(uniqueId);
+        }
+        if (fut == null) {
+            return;
+        }
+        while (true) {
+            try {
+                fut.get(10_000L, java.util.concurrent.TimeUnit.MILLISECONDS);
+                break;
+            } catch (InterruptedException ignored) {
+            } catch (java.util.concurrent.ExecutionException
+                     | java.util.concurrent.TimeoutException exception) {
+                LOGGER.warn("Failed to save player data for {}", playerName, exception);
+
+                String threadDump = "";
+                var threadMXBean = java.lang.management.ManagementFactory.getThreadMXBean();
+                for (var threadInfo : threadMXBean.dumpAllThreads(true, true)) {
+                    if (threadInfo.getThreadName().equals("Leaf IO Thread")) {
+                        threadDump = threadInfo.toString();
+                        break;
+                    }
+                }
+                LOGGER.warn(threadDump);
+                fut.cancel(true);
+                break;
+            } finally {
+                savingLocks.remove(uniqueId);
+            }
+        }
+    }
+    // Leaf end
+
     private void backup(String name, String stringUuid, String suffix) { // CraftBukkit
-@@ -60,7 +87,20 @@ public class PlayerDataStorage {
+         Path path = this.playerDir.toPath();
+         Path path1 = path.resolve(stringUuid + suffix); // CraftBukkit
+@@ -60,7 +124,13 @@ public class PlayerDataStorage {
         }
     }
 
@@ -134,19 +174,12 @@ index c44110b123ba5912af18faf0065e9ded780da9b7..541c440d3211c6625974ac1e0055d626
 +        return load(name, stringUuid, suffix, java.util.UUID.fromString(stringUuid));
 +    }
 +    private Optional<CompoundTag> load(String name, String stringUuid, String suffix, java.util.UUID playerUuid) { // CraftBukkit
-+        synchronized (PlayerDataStorage.this) {
-+            while (savingQueue.contains(playerUuid)) {
-+                try {
-+                    Thread.sleep(1L);
-+                } catch (InterruptedException ignored) {
-+                }
-+            }
-+        }
+        lockFor(playerUuid, name);
 +        // Leaf end - Async playerdata saving
         File file = new File(this.playerDir, stringUuid + suffix); // CraftBukkit
         // Spigot start
         boolean usingWrongFile = false;
-@@ -91,7 +131,7 @@ public class PlayerDataStorage {
+@@ -91,7 +161,7 @@ public class PlayerDataStorage {
 
     public Optional<CompoundTag> load(Player player) {
         // CraftBukkit start
@@ -155,7 +188,7 @@ index c44110b123ba5912af18faf0065e9ded780da9b7..541c440d3211c6625974ac1e0055d626
             if (player instanceof ServerPlayer serverPlayer) {
                 CraftPlayer craftPlayer = serverPlayer.getBukkitEntity();
                 // Only update first played if it is older than the one we have
-@@ -106,20 +146,25 @@ public class PlayerDataStorage {
+@@ -106,20 +176,25 @@ public class PlayerDataStorage {
         });
     }
 
--- a/leaf-server/paper-patches/features/0033-Async-playerdata-saving.patch
+++ b/leaf-server/paper-patches/features/0033-Async-playerdata-saving.patch
@@ -5,7 +5,7 @@ Subject: [PATCH] Async playerdata saving


 diff --git a/src/main/java/org/bukkit/craftbukkit/CraftOfflinePlayer.java b/src/main/java/org/bukkit/craftbukkit/CraftOfflinePlayer.java
-index f2d87c12dd19210ce7e2147fada5c10191008632..0cc6b6c597ca8c955b246764de637be4c25bfb40 100644
+index f2d87c12dd19210ce7e2147fada5c10191008632..bd6608787abcd1869f66d67297c6b4252193080a 100644
 --- a/src/main/java/org/bukkit/craftbukkit/CraftOfflinePlayer.java
 +++ b/src/main/java/org/bukkit/craftbukkit/CraftOfflinePlayer.java
@@ -207,7 +207,7 @@ public class CraftOfflinePlayer implements OfflinePlayer, ConfigurationSerializa
@@ -17,34 +17,23 @@ index f2d87c12dd19210ce7e2147fada5c10191008632..0cc6b6c597ca8c955b246764de637be4
     }
 
     private CompoundTag getBukkitData() {
-@@ -813,6 +813,17 @@ public class CraftOfflinePlayer implements OfflinePlayer, ConfigurationSerializa
+@@ -813,16 +813,9 @@ public class CraftOfflinePlayer implements OfflinePlayer, ConfigurationSerializa
      * @param compoundTag
      */
     private void save(CompoundTag compoundTag) {
+-        File playerDir = server.console.playerDataStorage.getPlayerDir();
+-        try {
+-            File tempFile = File.createTempFile(this.getUniqueId()+"-", ".dat", playerDir);
+-            net.minecraft.nbt.NbtIo.writeCompressed(compoundTag, tempFile.toPath());
+-            File playerDataFile = new File(playerDir, this.getUniqueId()+".dat");
+-            File playerDataFileOld = new File(playerDir, this.getUniqueId()+".dat_old");
+-            net.minecraft.Util.safeReplaceFile(playerDataFile.toPath(), tempFile.toPath(), playerDataFileOld.toPath());
+-        } catch (java.io.IOException e) {
+-            e.printStackTrace();
+-        }
 +        // Leaf start - Async playerdata saving
-+        synchronized (server.console.playerDataStorage) {
-+            while (server.console.playerDataStorage.savingQueue.contains(getUniqueId())) {
-+                try {
-+                    Thread.sleep(1L);
-+                } catch (InterruptedException ignored) {
-+                }
-+            }
-+            server.console.playerDataStorage.savingQueue.add(getUniqueId());
-+        }
-+        // Leaf end - Async playerdata saving
-         File playerDir = server.console.playerDataStorage.getPlayerDir();
-         try {
-             File tempFile = File.createTempFile(this.getUniqueId()+"-", ".dat", playerDir);
-@@ -822,6 +833,12 @@ public class CraftOfflinePlayer implements OfflinePlayer, ConfigurationSerializa
-             net.minecraft.Util.safeReplaceFile(playerDataFile.toPath(), tempFile.toPath(), playerDataFileOld.toPath());
-         } catch (java.io.IOException e) {
-             e.printStackTrace();
-+        // Leaf start - Async playerdata saving
-+        } finally {
-+            synchronized (server.console.playerDataStorage) {
-+                server.console.playerDataStorage.savingQueue.remove(getUniqueId());
-+            }
-+        // Leaf end - Async playerdata saving
-         }
+        server.console.playerDataStorage.save(this.getName(), this.getUniqueId(), this.getUniqueId().toString(), compoundTag);
+        // Leaf end
     }
     // Purpur end - OfflinePlayer API
+ }
--- a/leaf-server/src/main/java/org/dreeam/leaf/async/AsyncPlayerDataSaving.java
+++ b/leaf-server/src/main/java/org/dreeam/leaf/async/AsyncPlayerDataSaving.java
@@ -2,15 +2,13 @@ package org.dreeam.leaf.async;

 import org.dreeam.leaf.config.modules.async.AsyncPlayerDataSave;

-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
+import java.util.Optional;
+import java.util.concurrent.*;

 public class AsyncPlayerDataSaving {

    public static final ExecutorService IO_POOL = new ThreadPoolExecutor(
-        1, 1, 0, TimeUnit.MILLISECONDS,
+        1, 1, 0L, TimeUnit.MILLISECONDS,
        new LinkedBlockingQueue<>(),
        new com.google.common.util.concurrent.ThreadFactoryBuilder()
            .setPriority(Thread.NORM_PRIORITY - 2)
@@ -23,11 +21,12 @@ public class AsyncPlayerDataSaving {
    private AsyncPlayerDataSaving() {
    }

-    public static void save(Runnable runnable) {
+    public static Optional<Future<?>> submit(Runnable runnable) {
        if (!AsyncPlayerDataSave.enabled) {
            runnable.run();
+            return Optional.empty();
        } else {
-            IO_POOL.execute(runnable);
+            return Optional.of(IO_POOL.submit(runnable));
        }
    }
 }