Repository navigation
[BUG] <may MemorySafeWindowTinyLFUMap cause a memory leak> #4475
Description
Is there an existing issue for this?
- I have searched the existing issues
Current Behavior
memory leaked
Expected Behavior
when i use System.gc(), no memory leaked
Steps To Reproduce
public class TestMemoryLeak {
// 1m内存
private static final int M_1 = 1 * 1024 * 1024;
public static void main(String[] args) throws InterruptedException, IOException {
System.in.read();
try {
mswtMapTest();
} catch (Exception e) {
e.printStackTrace();
}
System.gc();
TimeUnit.SECONDS.sleep(Long.MAX_VALUE);
}
public static void mswtMapTest() {
for (int i = 0; i < 10; i++) {
System.gc();
MemorySafeWindowTinyLFUMap<String, Byte[]> map = new MemorySafeWindowTinyLFUMap<>(1, 100);
map.put(String.valueOf(i), new Byte[M_1 * 2]);
// 手动GC
}
}
public static void commonMapTest() {
for (int i = 0; i < 10; i++) {
System.gc();
Map<String, Byte[]> map = new HashMap<>();
map.put(String.valueOf(i), new Byte[M_1 * 2]);
// 手动GC
System.gc();
}
}
}
Environment
jvm： -Xms20m -Xmx20m
ShenYu version(s):Debug logs
Exception in thread "main" java.lang.OutOfMemoryError: Java heap space
at org.apache.shenyu.common.cache.TestMemoryLeak.mswtMapTest(TestMemoryLeak.java:44)
at org.apache.shenyu.common.cache.TestMemoryLeak.main(TestMemoryLeak.java:32)
Anything else?
MemorySafeWindowTinyLFUMap hold a static reference ALL, every reference of MemorySafeWindowTinyLFUMap will be always hold by the reference, and may cause a memory leak
Activity
yu199195 commentedon Mar 15, 2023
@xuyicheng1995 hi， can you fix it？
xuyicheng1995 commentedon Mar 15, 2023
ok i can have a try
moremind commentedon Mar 16, 2023
Caffeine won't catch OutOfMemoryError in order to suppress and evict entries. When that error is thrown the JVM is left in an unpredictable state, which may not be recoverable. It is best to use -XX:+ExitOnOutOfMemoryError and -XX:+HeapDumpOnOutOfMemoryError to restart and investigate the memory leak offline.
At first it might seem beneficial to defer all of the available memory to the caches and let the garbage collector evict when needed. This is the idea behind soft references. Unfortunately, that approach has been troublesome due to making full GCs more frequent and increasing their stop-the-world pause times. We instead recommend using an explicit maximum size, where the policy is quite intelligent. If you want large caches then usually a remote tier, like memcached or redis, is a good compliment so you get the speed of local access and a lower miss penalty on cache loads.
pls refer to:https://stackoverflow.com/questions/71401507/what-happens-when-caffeine-runs-out-of-memory
xuyicheng1995 commentedon Mar 16, 2023
sry, i think your reply has no relation with the question.
moremind commentedon Mar 16, 2023
org.apache.shenyu.common.cache.MemorySafeWindowTinyLFUMap#ALL is not related to oom, you can test it with caffeine, caffeine cannot rely on oom for cache clearing. so, no matter how you modify it, it will oom
moremind commentedon Mar 16, 2023
this is my test result with caffeine.
xuyicheng1995 commentedon Mar 16, 2023
when i new a instance of MemorySafeWindowTinyLFUMap, this instance will be hold by org.apache.shenyu.common.cache.MemorySafeWindowTinyLFUMap#ALL and current thread stack variable， i set the current thread variale to null，mean JVM will collect this instance，but this instance still is held by org.apache.shenyu.common.cache.MemorySafeWindowTinyLFUMap#ALL，JVM cannot collect this instance
xuyicheng1995 commentedon Mar 16, 2023
code like this：
xuyicheng1995 commentedon Mar 16, 2023
i think this code will cause memory leak, your test is about caffine cache, but caffine cache is held by a instance of MemorySafeWindowTinyLFUMap, so if org.apache.shenyu.common.cache.MemorySafeWindowTinyLFUMap#ALL do not remove this instance, gc not works
moremind commentedon Mar 16, 2023
use
org.apache.shenyu.common.cache.WindowTinyLFUMapto test, your code is a reference, you set the null, gc will work.xuyicheng1995 commentedon Mar 16, 2023
i dont understand, why let me use org.apache.shenyu.common.cache.WindowTinyLFUMap to test? what i said is, use MemorySafeWindowTinyLFUMap may cause a memory leak.
yunlongn commentedon Mar 16, 2023
But the
MemorySafeWindowTinyLFUMapobject is unique in God's Domain @xuyicheng1995ben-manes commentedon Mar 16, 2023
It looks like the request is to use a weak cache for the static
ALLfield so that instances may be collected when there are no strong references held by the application. As this is a static field that can be called by multiple threads, it should also be thread-safe. It doesn't look like ordering matters, so you could use a Caffeine weakKey cache withCollections.newSetFromMap. Something like,moremind commentedon Mar 16, 2023
as I know, Caffeine cannot be memory safe,
moremind commentedon Mar 16, 2023
or catch Out Of Memory Error to release caffeine cache.
moremind commentedon Mar 16, 2023
this may be refer to: ben-manes/caffeine#776, achieve memory safe is not easy for jvm.
ben-manes commentedon Mar 16, 2023
This is simply allowing the garbage collection of an instance that is no longer reachable by the client, but is held due to a private static field. By using weak references, you allow the instance to be collected once it becomes unused.
xuyicheng1995 commentedon Mar 16, 2023
i think that is a good idea, and I originally wanted to learn from Netty`s ReferenceCount class, user retain and release instance ref count, but it is a little diffcult
yu199195 commentedon Mar 17, 2023
@ben-manes @xuyicheng1995 Thanks for your suggestions, I think we have solved it~