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 82df3351ce..26110de0f2 100644 --- a/common/src/main/java/io/netty/util/concurrent/DefaultThreadFactory.java +++ b/common/src/main/java/io/netty/util/concurrent/DefaultThreadFactory.java @@ -97,8 +97,7 @@ public class DefaultThreadFactory implements ThreadFactory { } public DefaultThreadFactory(String poolName, boolean daemon, int priority) { - this(poolName, daemon, priority, System.getSecurityManager() == null ? - Thread.currentThread().getThreadGroup() : System.getSecurityManager().getThreadGroup()); + this(poolName, daemon, priority, null); } @Override diff --git a/common/src/test/java/io/netty/util/concurrent/DefaultThreadFactoryTest.java b/common/src/test/java/io/netty/util/concurrent/DefaultThreadFactoryTest.java index 0520e56630..ec3a881576 100644 --- a/common/src/test/java/io/netty/util/concurrent/DefaultThreadFactoryTest.java +++ b/common/src/test/java/io/netty/util/concurrent/DefaultThreadFactoryTest.java @@ -24,7 +24,9 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; public class DefaultThreadFactoryTest { @Test(timeout = 2000) @@ -112,24 +114,6 @@ public class DefaultThreadFactoryTest { sticky); } - // test that when DefaultThreadFactory is constructed it is sticky to the thread group from the thread group of the - // thread that created it - @Test(timeout = 2000) - public void testDefaultThreadFactoryInheritsThreadGroup() throws InterruptedException { - final ThreadGroup sticky = new ThreadGroup("sticky"); - - runStickyThreadGroupTest( - () -> { - final AtomicReference factory = - new AtomicReference<>(); - final Thread thread = new Thread(sticky, () -> factory.set(new DefaultThreadFactory("test"))); - thread.start(); - thread.join(); - return factory.get(); - }, - sticky); - } - // test that when a security manager is installed that provides a ThreadGroup, DefaultThreadFactory inherits from // the security manager @Test(timeout = 2000) @@ -216,4 +200,33 @@ public class DefaultThreadFactoryTest { assertEquals(secondGroup, secondCaptured.get()); } + + // test that when DefaultThreadFactory is constructed without a sticky thread group, threads + // created by it inherit the correct thread group + @Test(timeout = 2000) + public void testCurrentThreadGroupIsUsed() throws InterruptedException { + final AtomicReference factory = new AtomicReference<>(); + final AtomicReference firstCaptured = new AtomicReference<>(); + + final ThreadGroup group = new ThreadGroup("first"); + assertFalse(group.isDestroyed()); + final Thread first = new Thread(group, () -> { + final Thread current = Thread.currentThread(); + firstCaptured.set(current.getThreadGroup()); + factory.set(new DefaultThreadFactory("sticky", false)); + }); + first.start(); + first.join(); + // Destroy the group now + group.destroy(); + assertTrue(group.isDestroyed()); + assertEquals(group, firstCaptured.get()); + + ThreadGroup currentThreadGroup = Thread.currentThread().getThreadGroup(); + Thread second = factory.get().newThread(() -> { + // NOOP. + }); + second.join(); + assertEquals(currentThreadGroup, currentThreadGroup); + } }