From 8bdf73edea6cd70d2af19d5578487f771670da2b Mon Sep 17 00:00:00 2001 From: Kun Zhang Date: Tue, 5 Apr 2016 20:43:34 -0700 Subject: [PATCH] Explicit thread group on DefaultThreadFactory. Motivation: Fixes #5084. We (gRPC) encountered a bug that was triggered by grpc/grpc-java@d927180. After that commit, event loop threads are created per task by NioEventLoopGroup, and inherits the thread group of the caller, which in our case is an application-provided request-scope thread. Things go south when the application tries to manipulate (e.g., interrupt and join) all threads of the request-scope thread group, which unexpectedly include the event loop threads. Modifications: DefaultThreadFactory will save the current thread group in constructor, and apply it to all new threads. Result: Threads created by DefaultThreadFactory will be in the same thread group as the thread where the factory is created. --- .../netty/util/concurrent/DefaultThreadFactory.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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 84d72cdf24..bab2cf9c73 100644 --- a/common/src/main/java/io/netty/util/concurrent/DefaultThreadFactory.java +++ b/common/src/main/java/io/netty/util/concurrent/DefaultThreadFactory.java @@ -16,6 +16,7 @@ package io.netty.util.concurrent; +import io.netty.util.internal.ObjectUtil; import io.netty.util.internal.StringUtil; import java.util.Locale; @@ -33,6 +34,7 @@ public class DefaultThreadFactory implements ThreadFactory { private final String prefix; private final boolean daemon; private final int priority; + private final ThreadGroup threadGroup; public DefaultThreadFactory(Class poolType) { this(poolType, false, Thread.NORM_PRIORITY); @@ -82,7 +84,7 @@ public class DefaultThreadFactory implements ThreadFactory { } } - public DefaultThreadFactory(String poolName, boolean daemon, int priority) { + public DefaultThreadFactory(String poolName, boolean daemon, int priority, ThreadGroup threadGroup) { if (poolName == null) { throw new NullPointerException("poolName"); } @@ -94,6 +96,11 @@ public class DefaultThreadFactory implements ThreadFactory { prefix = poolName + '-' + poolId.incrementAndGet() + '-'; this.daemon = daemon; this.priority = priority; + this.threadGroup = ObjectUtil.checkNotNull(threadGroup, "threadGroup"); + } + + public DefaultThreadFactory(String poolName, boolean daemon, int priority) { + this(poolName, daemon, priority, Thread.currentThread().getThreadGroup()); } @Override @@ -119,8 +126,9 @@ public class DefaultThreadFactory implements ThreadFactory { return t; } + // TODO: Once we can break the API we should add ThreadGroup to the arguments of this method. protected Thread newThread(Runnable r, String name) { - return new FastThreadLocalThread(r, name); + return new FastThreadLocalThread(threadGroup, r, name); } private static final class DefaultRunnableDecorator implements Runnable {