Repository: netty/netty
Issue: Netty's BlockHound integration assumes `FastThreadLocalThread` is always non-blocking

### Expected behavior

The motivation is similar to https://github.com/netty/netty/issues/12125 which has already been reported and closed.

I'm wondering if `FastThreadLocalThread` can be used for executing blocking tasks while integrating with Netty's `BlockHound` integration.

### Actual behavior

Some libraries utilize `FastThreadLocalThread` not only for event loops but also for blocking task executors.
For this case, it is acceptable that a blocking task is executed within a `FastThreadLocalThread`.

However, Netty's `BlockHound` integration assumes that all `FastThreadLocalThread` instances should be non-blocking.

https://github.com/netty/netty/blob/cfcc5144578d3f998d353cc3c3b448fc56333e0d/common/src/main/java/io/netty/util/internal/Hidden.java#L173

I'm wondering if it would be possible to define a separate `FastThreadLocalThread` (e.g. `NonBlockingFastThreadLocalThread`) and apply `BlockHound` predicates on the `NonBlocking` variant instead.

I guess users may override `DefaultThreadFactory#newThread` and apply their own `FastThreadLocalThread` variant if they would like.

A potential diffset:

```
diff --git a/common/src/main/java/io/netty/util/concurrent/DefaultThreadFactory.java b/common/src/main/java/io/netty/util/concurrent/DefaultThreadFactory.java
index a18f236309..6e2b25f67c 100644
--- a/common/src/main/java/io/netty/util/concurrent/DefaultThreadFactory.java
+++ b/common/src/main/java/io/netty/util/concurrent/DefaultThreadFactory.java
@@ -118,6 +118,6 @@ public class DefaultThreadFactory implements ThreadFactory {
     }
 
     protected Thread newThread(Runnable r, String name) {
-        return new FastThreadLocalThread(threadGroup, r, name);
+        return new NonBlockingFastThreadLocalThread(threadGroup, r, name);
     }
 }
diff --git a/common/src/main/java/io/netty/util/concurrent/NonBlockingFastThreadLocalThread.java b/common/src/main/java/io/netty/util/concurrent/NonBlockingFastThreadLocalThread.java
new file mode 100644
index 0000000000..00b2455e43
--- /dev/null
+++ b/common/src/main/java/io/netty/util/concurrent/NonBlockingFastThreadLocalThread.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2022 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package io.netty.util.concurrent;
+
+/**
+ * A special {@link Thread} that provides fast access to {@link FastThreadLocal} variables.
+ */
+public final class NonBlockingFastThreadLocalThread extends FastThreadLocalThread {
+
+    public NonBlockingFastThreadLocalThread(ThreadGroup threadGroup, Runnable r, String name) {
+        super(threadGroup, r, name);
+    }
+
+    public NonBlockingFastThreadLocalThread(Runnable runnable) {
+        super(runnable);
+    }
+}
diff --git a/common/src/main/java/io/netty/util/internal/Hidden.java b/common/src/main/java/io/netty/util/internal/Hidden.java
index fec14d1254..4c67b7f166 100644
--- a/common/src/main/java/io/netty/util/internal/Hidden.java
+++ b/common/src/main/java/io/netty/util/internal/Hidden.java
@@ -16,13 +16,13 @@
 
 package io.netty.util.internal;
 
-import io.netty.util.concurrent.FastThreadLocalThread;
-import reactor.blockhound.BlockHound;
-import reactor.blockhound.integration.BlockHoundIntegration;
-
 import java.util.function.Function;
 import java.util.function.Predicate;
 
+import io.netty.util.concurrent.NonBlockingFastThreadLocalThread;
+import reactor.blockhound.BlockHound;
+import reactor.blockhound.integration.BlockHoundIntegration;
+
 /**
  * Contains classes that must have public visibility but are not public API.
  */
@@ -170,7 +170,7 @@ class Hidden {
                         @Override
                         @SuppressJava6Requirement(reason = "Predicate#test")
                         public boolean test(Thread thread) {
-                            return p.test(thread) || thread instanceof FastThreadLocalThread;
+                            return p.test(thread) || thread instanceof NonBlockingFastThreadLocalThread;
                         }
                     };
                 }
diff --git a/common/src/main/java/io/netty/util/internal/ObjectCleaner.java b/common/src/main/java/io/netty/util/internal/ObjectCleaner.java
index 0eb7b3495f..e1e76e6688 100644
--- a/common/src/main/java/io/netty/util/internal/ObjectCleaner.java
+++ b/common/src/main/java/io/netty/util/internal/ObjectCleaner.java
@@ -15,7 +15,8 @@
  */
 package io.netty.util.internal;
 
-import io.netty.util.concurrent.FastThreadLocalThread;
+import static io.netty.util.internal.SystemPropertyUtil.getInt;
+import static java.lang.Math.max;
 
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
@@ -24,8 +25,7 @@ import java.security.PrivilegedAction;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 
-import static io.netty.util.internal.SystemPropertyUtil.getInt;
-import static java.lang.Math.max;
+import io.netty.util.concurrent.NonBlockingFastThreadLocalThread;
 
 /**
  * Allows a way to register some {@link Runnable} that will executed once there are no references to an {@link Object}
@@ -100,7 +100,7 @@ public final class ObjectCleaner {
 
         // Check if there is already a cleaner running.
         if (CLEANER_RUNNING.compareAndSet(false, true)) {
-            final Thread cleanupThread = new FastThreadLocalThread(CLEANER_TASK);
+            final Thread cleanupThread = new NonBlockingFastThreadLocalThread(CLEANER_TASK);
             cleanupThread.setPriority(Thread.MIN_PRIORITY);
             // Set to null to ensure we not create classloader leaks by holding a strong reference to the inherited
             // classloader.


```

### Steps to reproduce

### Minimal yet complete reproducer code (or URL to code)

### Netty version

4.1.82

### JVM version (e.g. `java -version`)

n/a

### OS version (e.g. `uname -a`)

n/a

Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/concurrent/FastThreadLocalThread.java b/common/src/main/java/io/netty/util/concurrent/FastThreadLocalThread.java
index dcef6b28fda..9f7184ee9b5 100644
--- a/common/src/main/java/io/netty/util/concurrent/FastThreadLocalThread.java
+++ b/common/src/main/java/io/netty/util/concurrent/FastThreadLocalThread.java
@@ -111,4 +111,18 @@ public static boolean willCleanupFastThreadLocals(Thread thread) {
         return thread instanceof FastThreadLocalThread &&
                 ((FastThreadLocalThread) thread).willCleanupFastThreadLocals();
     }
+
+    /**
+     * Query whether this thread is allowed to perform blocking calls or not.
+     * {@link FastThreadLocalThread}s are often used in event-loops, where blocking calls are forbidden in order to
+     * prevent event-loop stalls, so this method returns {@code false} by default.
+     * <p>
+     * Subclasses of {@link FastThreadLocalThread} can override this method if they are not meant to be used for
+     * running event-loops.
+     *
+     * @return {@code false}, unless overriden by a subclass.
+     */
+    public boolean permitBlockingCalls() {
+        return false;
+    }
 }
diff --git a/common/src/main/java/io/netty/util/internal/Hidden.java b/common/src/main/java/io/netty/util/internal/Hidden.java
index fec14d1254b..9094fd677f9 100644
--- a/common/src/main/java/io/netty/util/internal/Hidden.java
+++ b/common/src/main/java/io/netty/util/internal/Hidden.java
@@ -170,7 +170,9 @@ public Predicate<Thread> apply(final Predicate<Thread> p) {
                         @Override
                         @SuppressJava6Requirement(reason = "Predicate#test")
                         public boolean test(Thread thread) {
-                            return p.test(thread) || thread instanceof FastThreadLocalThread;
+                            return p.test(thread) ||
+                                    thread instanceof FastThreadLocalThread &&
+                                            !((FastThreadLocalThread) thread).permitBlockingCalls();
                         }
                     };
                 }


Test patch:
diff --git a/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java b/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java
index 109543efe14..a005095d821 100644
--- a/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java
+++ b/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java
@@ -189,6 +189,23 @@ protected void run() {
         latch.await();
     }
 
+    @Test
+    void permittingBlockingCallsInFastThreadLocalThreadSubclass() throws Exception {
+        final FutureTask<Void> future = new FutureTask<>(() -> {
+            Thread.sleep(0);
+            return null;
+        });
+        FastThreadLocalThread thread = new FastThreadLocalThread(future) {
+            @Override
+            public boolean permitBlockingCalls() {
+                return true; // The Thread.sleep(0) call should not be flagged because we allow blocking calls.
+            }
+        };
+        thread.start();
+        future.get(5, TimeUnit.SECONDS);
+        thread.join();
+    }
+
     @Test
     @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
     public void testHashedWheelTimerStartStop() throws Exception {