diff --git a/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java b/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java index 3503748c0d..3279da73d9 100644 --- a/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java +++ b/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java @@ -219,6 +219,16 @@ final class PoolThreadCache { } } + /// TODO: In the future when we move to Java9+ we should use java.lang.ref.Cleaner. + @Override + protected void finalize() throws Throwable { + try { + super.finalize(); + } finally { + free(); + } + } + /** * Should be called if the Thread that uses this cache is about to exist to release resources out of the cache */ diff --git a/common/src/main/java/io/netty/util/Recycler.java b/common/src/main/java/io/netty/util/Recycler.java index e84adc5a9a..491464d9a3 100644 --- a/common/src/main/java/io/netty/util/Recycler.java +++ b/common/src/main/java/io/netty/util/Recycler.java @@ -17,7 +17,6 @@ package io.netty.util; import io.netty.util.concurrent.FastThreadLocal; -import io.netty.util.internal.ObjectCleaner; import io.netty.util.internal.SystemPropertyUtil; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -244,10 +243,9 @@ public abstract class Recycler { Link next; } - // This act as a place holder for the head Link but also will be used by the ObjectCleaner - // to return space that was before reserved. Its important this does not hold any reference to - // either Stack or WeakOrderQueue. - static final class Head implements Runnable { + // This act as a place holder for the head Link but also will reclaim space once finalized. + // Its important this does not hold any reference to either Stack or WeakOrderQueue. + static final class Head { private final AtomicInteger availableSharedCapacity; Link link; @@ -256,12 +254,21 @@ public abstract class Recycler { this.availableSharedCapacity = availableSharedCapacity; } + /// TODO: In the future when we move to Java9+ we should use java.lang.ref.Cleaner. @Override - public void run() { - Link head = link; - while (head != null) { - reclaimSpace(LINK_CAPACITY); - head = head.next; + protected void finalize() throws Throwable { + try { + super.finalize(); + } finally { + Link head = link; + link = null; + while (head != null) { + reclaimSpace(LINK_CAPACITY); + Link next = head.next; + // Unlink to help GC and guard against GC nepotism. + head.next = null; + head = next; + } } } @@ -318,12 +325,6 @@ public abstract class Recycler { // may be accessed while its still constructed. stack.setHead(queue); - // We need to reclaim all space that was reserved by this WeakOrderQueue so we not run out of space in - // the stack. This is needed as we not have a good life-time control over the queue as it is used in a - // WeakHashMap which will drop it at any time. - final Head head = queue.head; - ObjectCleaner.register(queue, head); - return queue; } diff --git a/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java b/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java index 561058abca..dacaa14100 100644 --- a/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java +++ b/common/src/main/java/io/netty/util/concurrent/FastThreadLocal.java @@ -16,7 +16,6 @@ package io.netty.util.concurrent; import io.netty.util.internal.InternalThreadLocalMap; -import io.netty.util.internal.ObjectCleaner; import io.netty.util.internal.PlatformDependent; import java.util.Collections; @@ -153,6 +152,8 @@ public class FastThreadLocal { threadLocalMap.setCleanerFlag(index); + // TODO: We need to find a better way to handle this. + /* // We will need to ensure we will trigger remove(InternalThreadLocalMap) so everything will be released // and FastThreadLocal.onRemoval(...) will be called. ObjectCleaner.register(current, new Runnable() { @@ -164,6 +165,7 @@ public class FastThreadLocal { // the Thread is collected by GC. In this case the ThreadLocal will be gone away already. } }); + */ } /** @@ -281,7 +283,9 @@ public class FastThreadLocal { } /** - * Invoked when this thread local variable is removed by {@link #remove()}. + * Invoked when this thread local variable is removed by {@link #remove()}. Be aware that {@link #remove()} + * is not guaranteed to be called when the `Thread` completes which means you can not depend on this for + * cleanup of the resources in the case of `Thread` completion. */ protected void onRemoval(@SuppressWarnings("UnusedParameters") V value) throws Exception { } } diff --git a/common/src/test/java/io/netty/util/concurrent/FastThreadLocalTest.java b/common/src/test/java/io/netty/util/concurrent/FastThreadLocalTest.java index 4551d4f78b..6457de297a 100644 --- a/common/src/test/java/io/netty/util/concurrent/FastThreadLocalTest.java +++ b/common/src/test/java/io/netty/util/concurrent/FastThreadLocalTest.java @@ -18,6 +18,7 @@ package io.netty.util.concurrent; import io.netty.util.internal.ObjectCleaner; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import java.util.concurrent.atomic.AtomicBoolean; @@ -96,13 +97,13 @@ public class FastThreadLocalTest { thread.start(); thread.join(); - assertEquals(1, ObjectCleaner.getLiveSetCount() - sizeWhenStart); + assertEquals(0, ObjectCleaner.getLiveSetCount() - sizeWhenStart); Thread thread2 = new Thread(runnable); thread2.start(); thread2.join(); - assertEquals(2, ObjectCleaner.getLiveSetCount() - sizeWhenStart); + assertEquals(0, ObjectCleaner.getLiveSetCount() - sizeWhenStart); } @Test @@ -128,13 +129,13 @@ public class FastThreadLocalTest { thread.start(); thread.join(); - assertEquals(2, ObjectCleaner.getLiveSetCount() - sizeWhenStart); + assertEquals(0, ObjectCleaner.getLiveSetCount() - sizeWhenStart); Thread thread2 = new Thread(runnable); thread2.start(); thread2.join(); - assertEquals(4, ObjectCleaner.getLiveSetCount() - sizeWhenStart); + assertEquals(0, ObjectCleaner.getLiveSetCount() - sizeWhenStart); } @Test(timeout = 4000) @@ -142,6 +143,7 @@ public class FastThreadLocalTest { testOnRemoveCalled(true, true); } + @Ignore("onRemoval(...) not called with non FastThreadLocal") @Test(timeout = 4000) public void testOnRemoveCalledForNonFastThreadLocalGet() throws Exception { testOnRemoveCalled(false, true); @@ -152,6 +154,7 @@ public class FastThreadLocalTest { testOnRemoveCalled(true, false); } + @Ignore("onRemoval(...) not called with non FastThreadLocal") @Test(timeout = 4000) public void testOnRemoveCalledForNonFastThreadLocalSet() throws Exception { testOnRemoveCalled(false, false);