Remove usage of ObjectCleaner (#8064)

Motivation:

ObjectCleaner does start a Thread to handle the cleaning of resources which leaks into the users application. We should not use it in netty itself to make things more predictable.

Modifications:

- Remove usage of ObjectCleaner and use finalize as a replacement when possible.
- Clarify javadocs for FastThreadLocal.onRemoval(...) to ensure its clear that remove() is not guaranteed to be called when the Thread completees and so this method is not enough to guarantee cleanup for this case.

Result:

Fixes https://github.com/netty/netty/issues/8017.

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
      */

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<T> {
             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<T> {
                 this.availableSharedCapacity = availableSharedCapacity;
             }
 
+            /// TODO: In the future when we move to Java9+ we should use java.lang.ref.Cleaner.
             @Override
-            public void run() {
+            protected void finalize() throws Throwable {
+                try {
+                    super.finalize();
+                } finally {
                     Link head = link;
+                    link = null;
                     while (head != null) {
                         reclaimSpace(LINK_CAPACITY);
-                    head = head.next;
+                        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<T> {
             // 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;
         }
 

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<V> {
 
         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<V> {
                 // the Thread is collected by GC. In this case the ThreadLocal will be gone away already.
             }
         });
+        */
     }
 
     /**
@@ -281,7 +283,9 @@ public class FastThreadLocal<V> {
     }
 
     /**
-     * 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 { }
 }

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);