From 922e4635240e1519d22edc0cb1ff9d2382464886 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 22 Mar 2019 12:16:21 +0100 Subject: [PATCH] Don't try to put back MemoryRegionCache.Entry objects into the Recycler when recycled because of a finalizer. (#8955) Motivation: In MemoryRegionCache.Entry we use the Recycler to reduce GC pressure and churn. The problem is that these will also be recycled when the PoolThreadCache is collected and finalize() is called. This then can have the effect that we try to load class but the WebApp is already stoped. This will produce an stacktrace like this on Tomcat: ``` 19-Mar-2019 15:53:21.351 INFO [Finalizer] org.apache.catalina.loader.WebappClassLoaderBase.checkStateForResourceLoading Illegal access: this web application instance has been stopped already. Could not load [java.util.WeakHashMap]. The following stack trace is thrown for debugging purposes as well as to attempt to terminate the thread which caused the illegal access. java.lang.IllegalStateException: Illegal access: this web application instance has been stopped already. Could not load [java.util.WeakHashMap]. The following stack trace is thrown for debugging purposes as well as to attempt to terminate the thread which caused the illegal access. at org.apache.catalina.loader.WebappClassLoaderBase.checkStateForResourceLoading(WebappClassLoaderBase.java:1383) at org.apache.catalina.loader.WebappClassLoaderBase.checkStateForClassLoading(WebappClassLoaderBase.java:1371) at org.apache.catalina.loader.WebappClassLoaderBase.loadClass(WebappClassLoaderBase.java:1224) at org.apache.catalina.loader.WebappClassLoaderBase.loadClass(WebappClassLoaderBase.java:1186) at io.netty.util.Recycler$3.initialValue(Recycler.java:233) at io.netty.util.Recycler$3.initialValue(Recycler.java:230) at io.netty.util.concurrent.FastThreadLocal.initialize(FastThreadLocal.java:188) at io.netty.util.concurrent.FastThreadLocal.get(FastThreadLocal.java:142) at io.netty.util.Recycler$Stack.pushLater(Recycler.java:624) at io.netty.util.Recycler$Stack.push(Recycler.java:597) at io.netty.util.Recycler$DefaultHandle.recycle(Recycler.java:225) at io.netty.buffer.PoolThreadCache$MemoryRegionCache$Entry.recycle(PoolThreadCache.java:478) at io.netty.buffer.PoolThreadCache$MemoryRegionCache.freeEntry(PoolThreadCache.java:459) at io.netty.buffer.PoolThreadCache$MemoryRegionCache.free(PoolThreadCache.java:430) at io.netty.buffer.PoolThreadCache$MemoryRegionCache.free(PoolThreadCache.java:422) at io.netty.buffer.PoolThreadCache.free(PoolThreadCache.java:279) at io.netty.buffer.PoolThreadCache.free(PoolThreadCache.java:270) at io.netty.buffer.PoolThreadCache.free(PoolThreadCache.java:241) at io.netty.buffer.PoolThreadCache.finalize(PoolThreadCache.java:230) at java.lang.System$2.invokeFinalize(System.java:1270) at java.lang.ref.Finalizer.runFinalizer(Finalizer.java:102) at java.lang.ref.Finalizer.access$100(Finalizer.java:34) at java.lang.ref.Finalizer$FinalizerThread.run(Finalizer.java:217) ``` Beside this we also need to ensure we not try to lazy load SizeClass when the finalizer is used as it may not be present anymore if the ClassLoader is already destroyed. This would produce an error like: ``` 20-Mar-2019 11:26:35.254 INFO [Finalizer] org.apache.catalina.loader.WebappClassLoaderBase.checkStateForResourceLoading Illegal access: this web application instance has been stopped already. Could not load [io.netty.buffer.PoolArena$1]. The following stack trace is thrown for debugging purposes as well as to attempt to terminate the thread which caused the illegal access. java.lang.IllegalStateException: Illegal access: this web application instance has been stopped already. Could not load [io.netty.buffer.PoolArena$1]. The following stack trace is thrown for debugging purposes as well as to attempt to terminate the thread which caused the illegal access. at org.apache.catalina.loader.WebappClassLoaderBase.checkStateForResourceLoading(WebappClassLoaderBase.java:1383) at org.apache.catalina.loader.WebappClassLoaderBase.checkStateForClassLoading(WebappClassLoaderBase.java:1371) at org.apache.catalina.loader.WebappClassLoaderBase.loadClass(WebappClassLoaderBase.java:1224) at org.apache.catalina.loader.WebappClassLoaderBase.loadClass(WebappClassLoaderBase.java:1186) at io.netty.buffer.PoolArena.freeChunk(PoolArena.java:287) at io.netty.buffer.PoolThreadCache$MemoryRegionCache.freeEntry(PoolThreadCache.java:464) at io.netty.buffer.PoolThreadCache$MemoryRegionCache.free(PoolThreadCache.java:429) at io.netty.buffer.PoolThreadCache$MemoryRegionCache.free(PoolThreadCache.java:421) at io.netty.buffer.PoolThreadCache.free(PoolThreadCache.java:278) at io.netty.buffer.PoolThreadCache.free(PoolThreadCache.java:269) at io.netty.buffer.PoolThreadCache.free(PoolThreadCache.java:240) at io.netty.buffer.PoolThreadCache.finalize(PoolThreadCache.java:229) at java.lang.System$2.invokeFinalize(System.java:1270) at java.lang.ref.Finalizer.runFinalizer(Finalizer.java:102) at java.lang.ref.Finalizer.access$100(Finalizer.java:34) at java.lang.ref.Finalizer$FinalizerThread.run(Finalizer.java:217) ``` Modifications: - Only try to put the Entry back into the Recycler if the PoolThredCache is not destroyed because of the finalizer. - Only try to access SizeClass if not triggered by finalizer. Result: No IllegalStateException anymoe when a webapp is reloaded in Tomcat that uses netty and uses the PooledByteBufAllocator. --- .../main/java/io/netty/buffer/PoolArena.java | 32 +++++++------ .../java/io/netty/buffer/PoolThreadCache.java | 45 ++++++++++--------- .../netty/buffer/PooledByteBufAllocator.java | 2 +- 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/PoolArena.java b/buffer/src/main/java/io/netty/buffer/PoolArena.java index 5f1ec79ffd..67955ce6d4 100644 --- a/buffer/src/main/java/io/netty/buffer/PoolArena.java +++ b/buffer/src/main/java/io/netty/buffer/PoolArena.java @@ -276,7 +276,7 @@ abstract class PoolArena implements PoolArenaMetric { return; } - freeChunk(chunk, handle, sizeClass, nioBuffer); + freeChunk(chunk, handle, sizeClass, nioBuffer, false); } } @@ -287,21 +287,25 @@ abstract class PoolArena implements PoolArenaMetric { return isTiny(normCapacity) ? SizeClass.Tiny : SizeClass.Small; } - void freeChunk(PoolChunk chunk, long handle, SizeClass sizeClass, ByteBuffer nioBuffer) { + void freeChunk(PoolChunk chunk, long handle, SizeClass sizeClass, ByteBuffer nioBuffer, boolean finalizer) { final boolean destroyChunk; synchronized (this) { - switch (sizeClass) { - case Normal: - ++deallocationsNormal; - break; - case Small: - ++deallocationsSmall; - break; - case Tiny: - ++deallocationsTiny; - break; - default: - throw new Error(); + // We only call this if freeChunk is not called because of the PoolThreadCache finalizer as otherwise this + // may fail due lazy class-loading in for example tomcat. + if (!finalizer) { + switch (sizeClass) { + case Normal: + ++deallocationsNormal; + break; + case Small: + ++deallocationsSmall; + break; + case Tiny: + ++deallocationsTiny; + break; + default: + throw new Error(); + } } destroyChunk = !chunk.parent.free(chunk, handle, nioBuffer); } diff --git a/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java b/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java index 01a69d5700..cb638f1a1d 100644 --- a/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java +++ b/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java @@ -227,23 +227,23 @@ final class PoolThreadCache { try { super.finalize(); } finally { - free(); + free(true); } } /** * Should be called if the Thread that uses this cache is about to exist to release resources out of the cache */ - void free() { + void free(boolean finalizer) { // As free() may be called either by the finalizer or by FastThreadLocal.onRemoval(...) we need to ensure // we only call this one time. if (freed.compareAndSet(false, true)) { - int numFreed = free(tinySubPageDirectCaches) + - free(smallSubPageDirectCaches) + - free(normalDirectCaches) + - free(tinySubPageHeapCaches) + - free(smallSubPageHeapCaches) + - free(normalHeapCaches); + int numFreed = free(tinySubPageDirectCaches, finalizer) + + free(smallSubPageDirectCaches, finalizer) + + free(normalDirectCaches, finalizer) + + free(tinySubPageHeapCaches, finalizer) + + free(smallSubPageHeapCaches, finalizer) + + free(normalHeapCaches, finalizer); if (numFreed > 0 && logger.isDebugEnabled()) { logger.debug("Freed {} thread-local buffer(s) from thread: {}", numFreed, @@ -260,23 +260,23 @@ final class PoolThreadCache { } } - private static int free(MemoryRegionCache[] caches) { + private static int free(MemoryRegionCache[] caches, boolean finalizer) { if (caches == null) { return 0; } int numFreed = 0; for (MemoryRegionCache c: caches) { - numFreed += free(c); + numFreed += free(c, finalizer); } return numFreed; } - private static int free(MemoryRegionCache cache) { + private static int free(MemoryRegionCache cache, boolean finalizer) { if (cache == null) { return 0; } - return cache.free(); + return cache.free(finalizer); } void trim() { @@ -418,16 +418,16 @@ final class PoolThreadCache { /** * Clear out this cache and free up all previous cached {@link PoolChunk}s and {@code handle}s. */ - public final int free() { - return free(Integer.MAX_VALUE); + public final int free(boolean finalizer) { + return free(Integer.MAX_VALUE, finalizer); } - private int free(int max) { + private int free(int max, boolean finalizer) { int numFreed = 0; for (; numFreed < max; numFreed++) { Entry entry = queue.poll(); if (entry != null) { - freeEntry(entry); + freeEntry(entry, finalizer); } else { // all cleared return numFreed; @@ -445,20 +445,23 @@ final class PoolThreadCache { // We not even allocated all the number that are if (free > 0) { - free(free); + free(free, false); } } @SuppressWarnings({ "unchecked", "rawtypes" }) - private void freeEntry(Entry entry) { + private void freeEntry(Entry entry, boolean finalizer) { PoolChunk chunk = entry.chunk; long handle = entry.handle; ByteBuffer nioBuffer = entry.nioBuffer; - // recycle now so PoolChunk can be GC'ed. - entry.recycle(); + if (!finalizer) { + // recycle now so PoolChunk can be GC'ed. This will only be done if this is not freed because of + // a finalizer. + entry.recycle(); + } - chunk.arena.freeChunk(chunk, handle, sizeClass, nioBuffer); + chunk.arena.freeChunk(chunk, handle, sizeClass, nioBuffer, finalizer); } static final class Entry { diff --git a/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java b/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java index 562070bc04..610d7aeb5e 100644 --- a/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java +++ b/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java @@ -474,7 +474,7 @@ public class PooledByteBufAllocator extends AbstractByteBufAllocator implements @Override protected void onRemoval(PoolThreadCache threadCache) { - threadCache.free(); + threadCache.free(false); } private PoolArena leastUsedArena(PoolArena[] arenas) {