From 6afab517b04e4aaf0c70e68e3b2888c06129197c Mon Sep 17 00:00:00 2001 From: Norman Maurer <norman_maurer@apple.com> Date: Mon, 9 Jul 2018 15:58:12 -0400 Subject: [PATCH] Guard against calling PoolThreadCache.free() multiple times. (#8108) Motivation: 5b1fe611a637c362a60b391079fff73b1a4ef912 introduced the usage of a finalizer as last resort for PoolThreadCache. As we may call free() from the FastThreadLocal.onRemoval(...) and finalize() we need to guard against multiple calls as otherwise we will corrupt internal state (that is used for metrics). Modifications: Use AtomicBoolean to guard against multiple calls of PoolThreadCache.free(). Result: No more corruption of internal state caused by calling PoolThreadCache.free() multuple times. --- .../java/io/netty/buffer/PoolThreadCache.java | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java b/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java index 3279da73d9..2ad397755e 100644 --- a/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java +++ b/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java @@ -27,6 +27,7 @@ import io.netty.util.internal.logging.InternalLoggerFactory; import java.nio.ByteBuffer; import java.util.Queue; +import java.util.concurrent.atomic.AtomicBoolean; /** * Acts a Thread cache for allocations. This implementation is moduled after @@ -54,6 +55,7 @@ final class PoolThreadCache { private final int numShiftsNormalDirect; private final int numShiftsNormalHeap; private final int freeSweepAllocationThreshold; + private final AtomicBoolean freed = new AtomicBoolean(); private int allocations; @@ -233,23 +235,28 @@ final class PoolThreadCache { * Should be called if the Thread that uses this cache is about to exist to release resources out of the cache */ void free() { - int numFreed = free(tinySubPageDirectCaches) + - free(smallSubPageDirectCaches) + - free(normalDirectCaches) + - free(tinySubPageHeapCaches) + - free(smallSubPageHeapCaches) + - free(normalHeapCaches); + // 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); - if (numFreed > 0 && logger.isDebugEnabled()) { - logger.debug("Freed {} thread-local buffer(s) from thread: {}", numFreed, Thread.currentThread().getName()); - } + if (numFreed > 0 && logger.isDebugEnabled()) { + logger.debug("Freed {} thread-local buffer(s) from thread: {}", numFreed, + Thread.currentThread().getName()); + } - if (directArena != null) { - directArena.numThreadCaches.getAndDecrement(); - } + if (directArena != null) { + directArena.numThreadCaches.getAndDecrement(); + } - if (heapArena != null) { - heapArena.numThreadCaches.getAndDecrement(); + if (heapArena != null) { + heapArena.numThreadCaches.getAndDecrement(); + } } }