From 6a439506b07605dd8566c86b672d3ee955715b51 Mon Sep 17 00:00:00 2001 From: Aikar Date: Sun, 16 Sep 2018 00:00:16 -0400 Subject: [PATCH] Fix major memory leaks in ExpiringMap computeIfAbsent would leak as the entry was never registered into the ttl map This fixes that ,as well as redesigns cleaning to not run on every manipulation, and instead to run clean once per tick per expiring map. diff --git a/src/main/java/net/minecraft/server/ExpiringMap.java b/src/main/java/net/minecraft/server/ExpiringMap.java index 4006f5a69c..49021d29ee 100644 --- a/src/main/java/net/minecraft/server/ExpiringMap.java +++ b/src/main/java/net/minecraft/server/ExpiringMap.java @@ -6,25 +6,113 @@ import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; import it.unimi.dsi.fastutil.longs.Long2LongMap.Entry; import it.unimi.dsi.fastutil.objects.ObjectIterator; import java.util.Map; +import java.util.function.BiFunction; +import java.util.function.Function; +import java.util.function.LongFunction; public class ExpiringMap extends Long2ObjectOpenHashMap { private final int a; - private final Long2LongMap b = new Long2LongLinkedOpenHashMap(); + private final Long2LongMap ttl = new Long2LongLinkedOpenHashMap(); // Paper public ExpiringMap(int i, int j) { super(i); this.a = j; } - private void a(long i) { - long j = SystemUtils.b(); - this.b.put(i, j); - ObjectIterator objectiterator = this.b.long2LongEntrySet().iterator(); + // Paper start + private synchronized void setAccess(long i) { a(i); } // Paper - OBFHELPER + private synchronized void a(long i) { + long j = System.currentTimeMillis(); // Paper + this.ttl.put(i, j); + if (!registered) { + registered = true; + MinecraftServer.getServer().expiringMaps.add(this); + } + } + + @Override + public T compute(long l, BiFunction biFunction) { + setAccess(l); + return super.compute(l, biFunction); + } + + @Override + public T putIfAbsent(long l, T t) { + setAccess(l); + return super.putIfAbsent(l, t); + } + + @Override + public T computeIfPresent(long l, BiFunction biFunction) { + setAccess(l); + return super.computeIfPresent(l, biFunction); + } + + @Override + public T computeIfAbsent(long l, LongFunction longFunction) { + setAccess(l); + return super.computeIfAbsent(l, longFunction); + } + + @Override + public boolean replace(long l, T t, T v1) { + setAccess(l); + return super.replace(l, t, v1); + } + + @Override + public T replace(long l, T t) { + setAccess(l); + return super.replace(l, t); + } + + @Override + public T putIfAbsent(Long aLong, T t) { + setAccess(aLong); + return super.putIfAbsent(aLong, t); + } + + @Override + public boolean replace(Long aLong, T t, T v1) { + setAccess(aLong); + return super.replace(aLong, t, v1); + } + + @Override + public T replace(Long aLong, T t) { + setAccess(aLong); + return super.replace(aLong, t); + } + + @Override + public T computeIfAbsent(Long aLong, Function function) { + setAccess(aLong); + return super.computeIfAbsent(aLong, function); + } + + @Override + public T computeIfPresent(Long aLong, BiFunction biFunction) { + setAccess(aLong); + return super.computeIfPresent(aLong, biFunction); + } + + @Override + public T compute(Long aLong, BiFunction biFunction) { + setAccess(aLong); + return compute(aLong, biFunction); + } + + private boolean registered = false; + + // Break clean to its own method to be ticked + synchronized boolean clean() { + long now = System.currentTimeMillis(); + ObjectIterator objectiterator = this.ttl.long2LongEntrySet().iterator(); // Paper while(objectiterator.hasNext()) { - Entry entry = (Entry)objectiterator.next(); - Object object = super.get(entry.getLongKey()); - if (j - entry.getLongValue() <= (long)this.a) { + Long2LongMap.Entry entry = objectiterator.next(); // Paper + T object = super.get(entry.getLongKey()); // Paper + if (now - entry.getLongValue() <= (long)this.a) { break; } @@ -33,7 +121,18 @@ public class ExpiringMap extends Long2ObjectOpenHashMap { objectiterator.remove(); } } - + if (this.ttl.size() != this.size()) { + MinecraftServer.LOGGER.error("WARNING: ExpiringMap desync - Memory leak risk!"); + for (Entry entry : this.long2ObjectEntrySet()) { + ttl.putIfAbsent(entry.getLongKey(), now); + } + } + if (isEmpty()) { + registered = false; + return true; + } + return false; + // Paper end } protected boolean a(T var1) { diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java index 80e8b023cf..70a609efcc 100644 --- a/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java @@ -155,6 +155,7 @@ public abstract class MinecraftServer implements IAsyncTaskHandler, IMojangStati public int autosavePeriod; public File bukkitDataPackFolder; public CommandDispatcher vanillaCommandDispatcher; + public List expiringMaps = java.util.Collections.synchronizedList(new java.util.ArrayList<>()); // PAper // CraftBukkit end // Spigot start public static final int TPS = 20; @@ -995,6 +996,7 @@ public abstract class MinecraftServer implements IAsyncTaskHandler, IMojangStati this.methodProfiler.e(); org.spigotmc.WatchdogThread.tick(); // Spigot PaperLightingQueue.processQueue(startTime); // Paper + expiringMaps.removeIf(ExpiringMap::clean); // Paper this.slackActivityAccountant.tickEnded(l); // Spigot co.aikar.timings.TimingsManager.FULL_SERVER_TICK.stopTiming(); // Paper } -- 2.19.0