Skip to content

Commit aef65fe

Browse files
WalshyDevWalshyDev
authored
Merge branch 'Slimefun:master' into master
2 parents 6986e5b + 5be4718 commit aef65fe

2 files changed

Lines changed: 68 additions & 6 deletions

File tree

‎src/main/java/io/github/thebusybiscuit/slimefun4/Threads.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package io.github.thebusybiscuit.slimefun4;
2+
3+
import javax.annotation.ParametersAreNonnullByDefault;
4+
5+
import org.bukkit.plugin.java.JavaPlugin;
6+
7+
public class Threads {
8+
9+
@ParametersAreNonnullByDefault
10+
public static void newThread(JavaPlugin plugin, String name, Runnable runnable) {
11+
// TODO: Change to thread pool
12+
new Thread(runnable, plugin.getName() + " - " + name).start();
13+
}
14+
15+
public static String getCaller() {
16+
// First item will be getting the call stack
17+
// Second item will be this call
18+
// Third item will be the func we care about being called
19+
// And finally will be the caller
20+
StackTraceElement element = Thread.currentThread().getStackTrace()[3];
21+
return element.getClassName() + "." + element.getMethodName() + ":" + element.getLineNumber();
22+
}
23+
}

‎src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
import java.util.Collection;
44
import java.util.Iterator;
55
import java.util.List;
6+
import java.util.Map;
67
import java.util.Optional;
78
import java.util.OptionalInt;
89
import java.util.Set;
910
import java.util.UUID;
11+
import java.util.concurrent.ConcurrentHashMap;
1012
import java.util.function.Consumer;
1113

1214
import javax.annotation.Nonnull;
@@ -28,6 +30,7 @@
2830
import io.github.bakedlibs.dough.common.ChatColors;
2931
import io.github.bakedlibs.dough.common.CommonPatterns;
3032
import io.github.bakedlibs.dough.config.Config;
33+
import io.github.thebusybiscuit.slimefun4.Threads;
3134
import io.github.thebusybiscuit.slimefun4.api.events.AsyncProfileLoadEvent;
3235
import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint;
3336
import io.github.thebusybiscuit.slimefun4.api.items.HashedArmorpiece;
@@ -55,6 +58,8 @@
5558
*/
5659
public class PlayerProfile {
5760

61+
private static final Map<UUID, Boolean> loading = new ConcurrentHashMap<>();
62+
5863
private final UUID ownerId;
5964
private final String name;
6065

@@ -361,17 +366,37 @@ public static boolean fromUUID(@Nonnull UUID uuid, @Nonnull Consumer<PlayerProfi
361366
*/
362367
public static boolean get(@Nonnull OfflinePlayer p, @Nonnull Consumer<PlayerProfile> callback) {
363368
Validate.notNull(p, "Cannot get a PlayerProfile for: null!");
364-
365369
UUID uuid = p.getUniqueId();
370+
371+
Debug.log(TestCase.PLAYER_PROFILE_DATA, "Getting PlayerProfile for {}", uuid);
372+
366373
PlayerProfile profile = Slimefun.getRegistry().getPlayerProfiles().get(uuid);
367374

368375
if (profile != null) {
376+
Debug.log(TestCase.PLAYER_PROFILE_DATA, "PlayerProfile for {} was already loaded", uuid);
369377
callback.accept(profile);
370378
return true;
371379
}
372380

373-
Bukkit.getScheduler().runTaskAsynchronously(Slimefun.instance(), () -> {
381+
// If we're already loading, we don't want to spin up a whole new thread and load the profile again/more
382+
// This can very easily cause CPU, memory and thread exhaustion if the profile is large
383+
// See #4011, #4116
384+
if (loading.containsKey(uuid)) {
385+
Debug.log(TestCase.PLAYER_PROFILE_DATA, "Attempted to get PlayerProfile ({}) while loading", uuid);
386+
Debug.log(TestCase.PLAYER_PROFILE_DATA, "Caller: {}", Threads.getCaller());
387+
388+
// We can't easily consume the callback so we will throw it away in this case
389+
// This will mean that if a user has attempted to do an action like open a block while
390+
// their profile is still loading. Instead of it opening after a second or whatever when the
391+
// profile is loaded, they will have to explicitly re-click the block/item/etc.
392+
// This isn't the best but I think it's totally reasonable.
393+
return false;
394+
}
395+
396+
loading.put(uuid, true);
397+
Threads.newThread(Slimefun.instance(), "PlayerProfile#get(" + uuid + ")", () -> {
374398
PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(p.getUniqueId());
399+
loading.remove(uuid);
375400

376401
AsyncProfileLoadEvent event = new AsyncProfileLoadEvent(new PlayerProfile(p, data));
377402
Bukkit.getPluginManager().callEvent(event);
@@ -394,14 +419,28 @@ public static boolean get(@Nonnull OfflinePlayer p, @Nonnull Consumer<PlayerProf
394419
*/
395420
public static boolean request(@Nonnull OfflinePlayer p) {
396421
Validate.notNull(p, "Cannot request a Profile for null");
422+
Debug.log(TestCase.PLAYER_PROFILE_DATA, "Requesting PlayerProfile for {}", p.getName());
423+
424+
UUID uuid = p.getUniqueId();
425+
426+
// If we're already loading, we don't want to spin up a whole new thread and load the profile again/more
427+
// This can very easily cause CPU, memory and thread exhaustion if the profile is large
428+
// See #4011, #4116
429+
if (loading.containsKey(uuid)) {
430+
Debug.log(TestCase.PLAYER_PROFILE_DATA, "Attempted to request PlayerProfile ({}) while loading", uuid);
431+
Debug.log(TestCase.PLAYER_PROFILE_DATA, "Caller: {}", Threads.getCaller());
432+
return false;
433+
}
397434

398-
if (!Slimefun.getRegistry().getPlayerProfiles().containsKey(p.getUniqueId())) {
435+
if (!Slimefun.getRegistry().getPlayerProfiles().containsKey(uuid)) {
436+
loading.put(uuid, true);
399437
// Should probably prevent multiple requests for the same profile in the future
400-
Bukkit.getScheduler().runTaskAsynchronously(Slimefun.instance(), () -> {
401-
PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(p.getUniqueId());
438+
Threads.newThread(Slimefun.instance(), "PlayerProfile#request(" + uuid + ")", () -> {
439+
PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(uuid);
440+
loading.remove(uuid);
402441

403442
PlayerProfile pp = new PlayerProfile(p, data);
404-
Slimefun.getRegistry().getPlayerProfiles().put(p.getUniqueId(), pp);
443+
Slimefun.getRegistry().getPlayerProfiles().put(uuid, pp);
405444
});
406445

407446
return false;

0 commit comments

Comments
 (0)