diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/Threads.java b/src/main/java/io/github/thebusybiscuit/slimefun4/Threads.java new file mode 100644 index 00000000000..d109bcae9dd --- /dev/null +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/Threads.java @@ -0,0 +1,23 @@ +package io.github.thebusybiscuit.slimefun4; + +import javax.annotation.ParametersAreNonnullByDefault; + +import org.bukkit.plugin.java.JavaPlugin; + +public class Threads { + + @ParametersAreNonnullByDefault + public static void newThread(JavaPlugin plugin, String name, Runnable runnable) { + // TODO: Change to thread pool + new Thread(runnable, plugin.getName() + " - " + name).start(); + } + + public static String getCaller() { + // First item will be getting the call stack + // Second item will be this call + // Third item will be the func we care about being called + // And finally will be the caller + StackTraceElement element = Thread.currentThread().getStackTrace()[3]; + return element.getClassName() + "." + element.getMethodName() + ":" + element.getLineNumber(); + } +} diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index 7185471363c..de5432fd1b3 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -3,10 +3,12 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.OptionalInt; import java.util.Set; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import javax.annotation.Nonnull; @@ -28,6 +30,7 @@ import io.github.bakedlibs.dough.common.ChatColors; import io.github.bakedlibs.dough.common.CommonPatterns; import io.github.bakedlibs.dough.config.Config; +import io.github.thebusybiscuit.slimefun4.Threads; import io.github.thebusybiscuit.slimefun4.api.events.AsyncProfileLoadEvent; import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint; import io.github.thebusybiscuit.slimefun4.api.items.HashedArmorpiece; @@ -55,6 +58,8 @@ */ public class PlayerProfile { + private static final Map loading = new ConcurrentHashMap<>(); + private final UUID ownerId; private final String name; @@ -361,17 +366,37 @@ public static boolean fromUUID(@Nonnull UUID uuid, @Nonnull Consumer callback) { Validate.notNull(p, "Cannot get a PlayerProfile for: null!"); - UUID uuid = p.getUniqueId(); + + Debug.log(TestCase.PLAYER_PROFILE_DATA, "Getting PlayerProfile for {}", uuid); + PlayerProfile profile = Slimefun.getRegistry().getPlayerProfiles().get(uuid); if (profile != null) { + Debug.log(TestCase.PLAYER_PROFILE_DATA, "PlayerProfile for {} was already loaded", uuid); callback.accept(profile); return true; } - Bukkit.getScheduler().runTaskAsynchronously(Slimefun.instance(), () -> { + // If we're already loading, we don't want to spin up a whole new thread and load the profile again/more + // This can very easily cause CPU, memory and thread exhaustion if the profile is large + // See #4011, #4116 + if (loading.containsKey(uuid)) { + Debug.log(TestCase.PLAYER_PROFILE_DATA, "Attempted to get PlayerProfile ({}) while loading", uuid); + Debug.log(TestCase.PLAYER_PROFILE_DATA, "Caller: {}", Threads.getCaller()); + + // We can't easily consume the callback so we will throw it away in this case + // This will mean that if a user has attempted to do an action like open a block while + // their profile is still loading. Instead of it opening after a second or whatever when the + // profile is loaded, they will have to explicitly re-click the block/item/etc. + // This isn't the best but I think it's totally reasonable. + return false; + } + + loading.put(uuid, true); + Threads.newThread(Slimefun.instance(), "PlayerProfile#get(" + uuid + ")", () -> { PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(p.getUniqueId()); + loading.remove(uuid); AsyncProfileLoadEvent event = new AsyncProfileLoadEvent(new PlayerProfile(p, data)); Bukkit.getPluginManager().callEvent(event); @@ -394,14 +419,28 @@ public static boolean get(@Nonnull OfflinePlayer p, @Nonnull Consumer { - PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(p.getUniqueId()); + Threads.newThread(Slimefun.instance(), "PlayerProfile#request(" + uuid + ")", () -> { + PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(uuid); + loading.remove(uuid); PlayerProfile pp = new PlayerProfile(p, data); - Slimefun.getRegistry().getPlayerProfiles().put(p.getUniqueId(), pp); + Slimefun.getRegistry().getPlayerProfiles().put(uuid, pp); }); return false;