Fix GameProfileCache concurrency

Separate lookup and state access locks prevent lookups
from stalling simple state access/write calls
This commit is contained in:
Spottedleaf 2020-07-11 05:09:28 -07:00
parent 7b293aba08
commit efb3bbf2bb

View file

@ -1,6 +1,32 @@
--- a/net/minecraft/server/players/GameProfileCache.java --- a/net/minecraft/server/players/GameProfileCache.java
+++ b/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<GameProfile> lookupGameProfile(GameProfileRepository repository, String name) {
@@ -85,10 +92,12 @@
} }
public void onProfileLookupFailed(String s1, Exception exception) { public void onProfileLookupFailed(String s1, Exception exception) {
@ -14,7 +40,7 @@
repository.findProfilesByNames(new String[]{name}, profilelookupcallback); repository.findProfilesByNames(new String[]{name}, profilelookupcallback);
GameProfile gameprofile = (GameProfile) atomicreference.get(); GameProfile gameprofile = (GameProfile) atomicreference.get();
@@ -105,7 +107,7 @@ @@ -105,7 +114,7 @@
} }
private static boolean usesAuthentication() { private static boolean usesAuthentication() {
@ -23,7 +49,7 @@
} }
public void add(GameProfile profile) { public void add(GameProfile profile) {
@@ -117,13 +119,24 @@ @@ -117,15 +126,29 @@
GameProfileCache.GameProfileInfo usercache_usercacheentry = new GameProfileCache.GameProfileInfo(profile, date); GameProfileCache.GameProfileInfo usercache_usercacheentry = new GameProfileCache.GameProfileInfo(profile, date);
this.safeAdd(usercache_usercacheentry); this.safeAdd(usercache_usercacheentry);
@ -37,24 +63,34 @@
+ // Paper start + // Paper start
+ public @Nullable GameProfile getProfileIfCached(String name) { + public @Nullable GameProfile getProfileIfCached(String name) {
+ try { this.stateLock.lock(); // Paper - Fix GameProfileCache concurrency
+ GameProfileCache.GameProfileInfo entry = this.profilesByName.get(name.toLowerCase(Locale.ROOT)); + GameProfileCache.GameProfileInfo entry = this.profilesByName.get(name.toLowerCase(Locale.ROOT));
+ if (entry == null) { + if (entry == null) {
+ return null; + return null;
+ } + }
+ entry.setLastAccess(this.getNextOperation()); + entry.setLastAccess(this.getNextOperation());
+ return entry.getProfile(); + return entry.getProfile();
+ } finally { this.stateLock.unlock(); } // Paper - Fix GameProfileCache concurrency
+ } + }
+ // Paper end + // Paper end
+ +
public Optional<GameProfile> get(String name) { public Optional<GameProfile> get(String name) {
String s1 = name.toLowerCase(Locale.ROOT); 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); 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()); usercache_usercacheentry.setLastAccess(this.getNextOperation());
optional = Optional.of(usercache_usercacheentry.getProfile()); optional = Optional.of(usercache_usercacheentry.getProfile());
+ stateLocked = false; this.stateLock.unlock(); // Paper - Fix GameProfileCache concurrency
} else { } else {
- optional = GameProfileCache.lookupGameProfile(this.profileRepository, s1); - 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 + optional = GameProfileCache.lookupGameProfile(this.profileRepository, name); // CraftBukkit - use correct case for offline players
+ } finally { this.lookupLock.unlock(); } // Paper - Fix GameProfileCache concurrency
if (optional.isPresent()) { if (optional.isPresent()) {
this.add((GameProfile) optional.get()); this.add((GameProfile) optional.get());
flag = false; flag = false;
@ -68,7 +104,11 @@
} }
return optional; return optional;
@@ -167,7 +180,7 @@ + } finally { if (stateLocked) { this.stateLock.unlock(); } } // Paper - Fix GameProfileCache concurrency
}
public CompletableFuture<Optional<GameProfile>> getAsync(String username) {
@@ -167,7 +195,7 @@
} else { } else {
CompletableFuture<Optional<GameProfile>> completablefuture1 = CompletableFuture.supplyAsync(() -> { CompletableFuture<Optional<GameProfile>> completablefuture1 = CompletableFuture.supplyAsync(() -> {
return this.get(username); return this.get(username);
@ -77,7 +117,23 @@
this.requests.remove(username); this.requests.remove(username);
}, this.executor); }, this.executor);
@@ -208,7 +221,7 @@ @@ -178,6 +206,7 @@
}
public Optional<GameProfile> 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: label54:
{ {
@ -86,7 +142,7 @@
try { try {
JsonArray jsonarray = (JsonArray) this.gson.fromJson(bufferedreader, JsonArray.class); JsonArray jsonarray = (JsonArray) this.gson.fromJson(bufferedreader, JsonArray.class);
@@ -217,7 +230,7 @@ @@ -217,7 +247,7 @@
DateFormat dateformat = GameProfileCache.createDateFormat(); DateFormat dateformat = GameProfileCache.createDateFormat();
jsonarray.forEach((jsonelement) -> { jsonarray.forEach((jsonelement) -> {
@ -95,7 +151,7 @@
Objects.requireNonNull(list); Objects.requireNonNull(list);
optional.ifPresent(list::add); optional.ifPresent(list::add);
@@ -250,6 +263,11 @@ @@ -250,6 +280,11 @@
} }
} catch (FileNotFoundException filenotfoundexception) { } catch (FileNotFoundException filenotfoundexception) {
; ;
@ -107,7 +163,7 @@
} catch (JsonParseException | IOException ioexception) { } catch (JsonParseException | IOException ioexception) {
GameProfileCache.LOGGER.warn("Failed to load profile cache {}", this.file, ioexception); GameProfileCache.LOGGER.warn("Failed to load profile cache {}", this.file, ioexception);
} }
@@ -257,14 +275,15 @@ @@ -257,14 +292,15 @@
return list; return list;
} }
@ -117,7 +173,7 @@
DateFormat dateformat = GameProfileCache.createDateFormat(); DateFormat dateformat = GameProfileCache.createDateFormat();
- this.getTopMRUProfiles(1000).forEach((usercache_usercacheentry) -> { - 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)); jsonarray.add(GameProfileCache.writeGameProfile(usercache_usercacheentry, dateformat));
}); });
String s = this.gson.toJson(jsonarray); String s = this.gson.toJson(jsonarray);
@ -125,7 +181,7 @@
try { try {
BufferedWriter bufferedwriter = Files.newWriter(this.file, StandardCharsets.UTF_8); BufferedWriter bufferedwriter = Files.newWriter(this.file, StandardCharsets.UTF_8);
@@ -289,6 +308,14 @@ @@ -289,13 +325,32 @@
} catch (IOException ioexception) { } catch (IOException ioexception) {
; ;
} }
@ -140,3 +196,22 @@
} }
private Stream<GameProfileCache.GameProfileInfo> 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<GameProfileCache.GameProfileInfo> 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();