From efb3bbf2bb950ec843ad5185aca73e401e77ddd0 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Sat, 11 Jul 2020 05:09:28 -0700 Subject: [PATCH] Fix GameProfileCache concurrency Separate lookup and state access locks prevent lookups from stalling simple state access/write calls --- .../players/GameProfileCache.java.patch | 97 ++++++++++++++++--- 1 file changed, 86 insertions(+), 11 deletions(-) diff --git a/paper-server/patches/sources/net/minecraft/server/players/GameProfileCache.java.patch b/paper-server/patches/sources/net/minecraft/server/players/GameProfileCache.java.patch index b1338392e4..8b31e2142b 100644 --- a/paper-server/patches/sources/net/minecraft/server/players/GameProfileCache.java.patch +++ b/paper-server/patches/sources/net/minecraft/server/players/GameProfileCache.java.patch @@ -1,6 +1,32 @@ --- a/net/minecraft/server/players/GameProfileCache.java +++ b/net/minecraft/server/players/GameProfileCache.java -@@ -85,10 +85,12 @@ +@@ -60,6 +60,11 @@ + @Nullable + private Executor executor; + ++ // Paper start - Fix GameProfileCache concurrency ++ protected final java.util.concurrent.locks.ReentrantLock stateLock = new java.util.concurrent.locks.ReentrantLock(); ++ protected final java.util.concurrent.locks.ReentrantLock lookupLock = new java.util.concurrent.locks.ReentrantLock(); ++ // Paper end - Fix GameProfileCache concurrency ++ + public GameProfileCache(GameProfileRepository profileRepository, File cacheFile) { + this.profileRepository = profileRepository; + this.file = cacheFile; +@@ -67,11 +72,13 @@ + } + + private void safeAdd(GameProfileCache.GameProfileInfo entry) { ++ try { this.stateLock.lock(); // Paper - Fix GameProfileCache concurrency + GameProfile gameprofile = entry.getProfile(); + + entry.setLastAccess(this.getNextOperation()); + this.profilesByName.put(gameprofile.getName().toLowerCase(Locale.ROOT), entry); + this.profilesByUUID.put(gameprofile.getId(), entry); ++ } finally { this.stateLock.unlock(); } // Paper - Fix GameProfileCache concurrency + } + + private static Optional lookupGameProfile(GameProfileRepository repository, String name) { +@@ -85,10 +92,12 @@ } public void onProfileLookupFailed(String s1, Exception exception) { @@ -14,7 +40,7 @@ repository.findProfilesByNames(new String[]{name}, profilelookupcallback); GameProfile gameprofile = (GameProfile) atomicreference.get(); -@@ -105,7 +107,7 @@ +@@ -105,7 +114,7 @@ } private static boolean usesAuthentication() { @@ -23,7 +49,7 @@ } public void add(GameProfile profile) { -@@ -117,13 +119,24 @@ +@@ -117,15 +126,29 @@ GameProfileCache.GameProfileInfo usercache_usercacheentry = new GameProfileCache.GameProfileInfo(profile, date); this.safeAdd(usercache_usercacheentry); @@ -37,24 +63,34 @@ + // Paper start + public @Nullable GameProfile getProfileIfCached(String name) { ++ try { this.stateLock.lock(); // Paper - Fix GameProfileCache concurrency + GameProfileCache.GameProfileInfo entry = this.profilesByName.get(name.toLowerCase(Locale.ROOT)); + if (entry == null) { + return null; + } + entry.setLastAccess(this.getNextOperation()); + return entry.getProfile(); ++ } finally { this.stateLock.unlock(); } // Paper - Fix GameProfileCache concurrency + } + // Paper end + public Optional get(String name) { String s1 = name.toLowerCase(Locale.ROOT); ++ boolean stateLocked = true; try { this.stateLock.lock(); // Paper - Fix GameProfileCache concurrency GameProfileCache.GameProfileInfo usercache_usercacheentry = (GameProfileCache.GameProfileInfo) this.profilesByName.get(s1); -@@ -142,15 +155,15 @@ + boolean flag = false; + +@@ -141,19 +164,24 @@ + if (usercache_usercacheentry != null) { usercache_usercacheentry.setLastAccess(this.getNextOperation()); optional = Optional.of(usercache_usercacheentry.getProfile()); ++ stateLocked = false; this.stateLock.unlock(); // Paper - Fix GameProfileCache concurrency } else { - optional = GameProfileCache.lookupGameProfile(this.profileRepository, s1); ++ stateLocked = false; this.stateLock.unlock(); // Paper - Fix GameProfileCache concurrency ++ try { this.lookupLock.lock(); // Paper - Fix GameProfileCache concurrency + optional = GameProfileCache.lookupGameProfile(this.profileRepository, name); // CraftBukkit - use correct case for offline players ++ } finally { this.lookupLock.unlock(); } // Paper - Fix GameProfileCache concurrency if (optional.isPresent()) { this.add((GameProfile) optional.get()); flag = false; @@ -68,7 +104,11 @@ } return optional; -@@ -167,7 +180,7 @@ ++ } finally { if (stateLocked) { this.stateLock.unlock(); } } // Paper - Fix GameProfileCache concurrency + } + + public CompletableFuture> getAsync(String username) { +@@ -167,7 +195,7 @@ } else { CompletableFuture> completablefuture1 = CompletableFuture.supplyAsync(() -> { return this.get(username); @@ -77,7 +117,23 @@ this.requests.remove(username); }, this.executor); -@@ -208,7 +221,7 @@ +@@ -178,6 +206,7 @@ + } + + public Optional get(UUID uuid) { ++ try { this.stateLock.lock(); // Paper - Fix GameProfileCache concurrency + GameProfileCache.GameProfileInfo usercache_usercacheentry = (GameProfileCache.GameProfileInfo) this.profilesByUUID.get(uuid); + + if (usercache_usercacheentry == null) { +@@ -186,6 +215,7 @@ + usercache_usercacheentry.setLastAccess(this.getNextOperation()); + return Optional.of(usercache_usercacheentry.getProfile()); + } ++ } finally { this.stateLock.unlock(); } // Paper - Fix GameProfileCache concurrency + } + + public void setExecutor(Executor executor) { +@@ -208,7 +238,7 @@ label54: { @@ -86,7 +142,7 @@ try { JsonArray jsonarray = (JsonArray) this.gson.fromJson(bufferedreader, JsonArray.class); -@@ -217,7 +230,7 @@ +@@ -217,7 +247,7 @@ DateFormat dateformat = GameProfileCache.createDateFormat(); jsonarray.forEach((jsonelement) -> { @@ -95,7 +151,7 @@ Objects.requireNonNull(list); optional.ifPresent(list::add); -@@ -250,6 +263,11 @@ +@@ -250,6 +280,11 @@ } } catch (FileNotFoundException filenotfoundexception) { ; @@ -107,7 +163,7 @@ } catch (JsonParseException | IOException ioexception) { GameProfileCache.LOGGER.warn("Failed to load profile cache {}", this.file, ioexception); } -@@ -257,14 +275,15 @@ +@@ -257,14 +292,15 @@ return list; } @@ -117,7 +173,7 @@ DateFormat dateformat = GameProfileCache.createDateFormat(); - this.getTopMRUProfiles(1000).forEach((usercache_usercacheentry) -> { -+ this.getTopMRUProfiles(org.spigotmc.SpigotConfig.userCacheCap).forEach((usercache_usercacheentry) -> { // Spigot ++ this.listTopMRUProfiles(org.spigotmc.SpigotConfig.userCacheCap).forEach((usercache_usercacheentry) -> { // Spigot // Paper - Fix GameProfileCache concurrency jsonarray.add(GameProfileCache.writeGameProfile(usercache_usercacheentry, dateformat)); }); String s = this.gson.toJson(jsonarray); @@ -125,7 +181,7 @@ try { BufferedWriter bufferedwriter = Files.newWriter(this.file, StandardCharsets.UTF_8); -@@ -289,6 +308,14 @@ +@@ -289,13 +325,32 @@ } catch (IOException ioexception) { ; } @@ -140,3 +196,22 @@ } + private Stream getTopMRUProfiles(int limit) { +- return ImmutableList.copyOf(this.profilesByUUID.values()).stream().sorted(Comparator.comparing(GameProfileCache.GameProfileInfo::getLastAccess).reversed()).limit((long) limit); ++ // Paper start - Fix GameProfileCache concurrency ++ return this.listTopMRUProfiles(limit).stream(); + } + ++ private List listTopMRUProfiles(int limit) { ++ try { ++ this.stateLock.lock(); ++ return this.profilesByUUID.values().stream().sorted(Comparator.comparing(GameProfileCache.GameProfileInfo::getLastAccess).reversed()).limit(limit).toList(); ++ } finally { ++ this.stateLock.unlock(); ++ } ++ } ++ // Paper end - Fix GameProfileCache concurrency ++ + private static JsonElement writeGameProfile(GameProfileCache.GameProfileInfo entry, DateFormat dateFormat) { + JsonObject jsonobject = new JsonObject(); +