From f5f7e6f9e5a4c0f01753ff56869c903e635bb472 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Thu, 25 Jan 2018 13:48:59 +0100
Subject: [PATCH] ObjectCleanerThread must be a deamon thread to ensure the JVM
 can always terminate.

Motivation:

The ObjectCleanerThread must be a daemon thread as otherwise we may block the JVM from exit. By using a daemon thread we basically give the same garantees as the JVM when it comes to cleanup of resources (as the GC threads are also daemon threads and the CleanerImpl uses a deamon thread as well in Java9+).

Modifications:

Change ObjectCleanThread to be a daemon thread.

Result:

JVM shutdown will always be able to complete. Fixed [#7617].
---
 .../io/netty/util/internal/ObjectCleaner.java | 12 +++++-----
 .../util/internal/ObjectCleanerTest.java      | 24 +++++++++++++++++++
 2 files changed, 30 insertions(+), 6 deletions(-)

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 96d0443956..991f30d0dc 100644
--- a/common/src/main/java/io/netty/util/internal/ObjectCleaner.java
+++ b/common/src/main/java/io/netty/util/internal/ObjectCleaner.java
@@ -16,8 +16,6 @@
 package io.netty.util.internal;
 
 import io.netty.util.concurrent.FastThreadLocalThread;
-import io.netty.util.internal.logging.InternalLogger;
-import io.netty.util.internal.logging.InternalLoggerFactory;
 
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.WeakReference;
@@ -36,11 +34,13 @@ import static java.lang.Math.max;
 public final class ObjectCleaner {
     private static final int REFERENCE_QUEUE_POLL_TIMEOUT_MS =
             max(500, getInt("io.netty.util.internal.ObjectCleaner.refQueuePollTimeout", 10000));
+
+    // Package-private for testing
+    static final String CLEANER_THREAD_NAME = ObjectCleaner.class.getSimpleName() + "Thread";
     // This will hold a reference to the AutomaticCleanerReference which will be removed once we called cleanup()
     private static final Set<AutomaticCleanerReference> LIVE_SET = new ConcurrentSet<AutomaticCleanerReference>();
     private static final ReferenceQueue<Object> REFERENCE_QUEUE = new ReferenceQueue<Object>();
     private static final AtomicBoolean CLEANER_RUNNING = new AtomicBoolean(false);
-    private static final String CLEANER_THREAD_NAME = ObjectCleaner.class.getSimpleName() + "Thread";
     private static final Runnable CLEANER_TASK = new Runnable() {
         @Override
         public void run() {
@@ -116,9 +116,9 @@ public final class ObjectCleaner {
             });
             cleanupThread.setName(CLEANER_THREAD_NAME);
 
-            // This Thread is not a daemon as it will die once all references to the registered Objects will go away
-            // and its important to always invoke all cleanup tasks as these may free up direct memory etc.
-            cleanupThread.setDaemon(false);
+            // Mark this as a daemon thread to ensure that we the JVM can exit if this is the only thread that is
+            // running.
+            cleanupThread.setDaemon(true);
             cleanupThread.start();
         }
     }
diff --git a/common/src/test/java/io/netty/util/internal/ObjectCleanerTest.java b/common/src/test/java/io/netty/util/internal/ObjectCleanerTest.java
index 0db3e26945..7e75504021 100644
--- a/common/src/test/java/io/netty/util/internal/ObjectCleanerTest.java
+++ b/common/src/test/java/io/netty/util/internal/ObjectCleanerTest.java
@@ -23,6 +23,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
 
 public class ObjectCleanerTest {
 
@@ -110,4 +112,26 @@ public class ObjectCleanerTest {
             Thread.sleep(100);
         }
     }
+
+    @Test(timeout = 5000)
+    public void testCleanerThreadIsDaemon() throws Exception {
+        temporaryObject = new Object();
+        ObjectCleaner.register(temporaryObject, new Runnable() {
+            @Override
+            public void run() {
+                // NOOP
+            }
+        });
+
+        Thread cleanerThread = null;
+
+        for (Thread thread : Thread.getAllStackTraces().keySet()) {
+            if (thread.getName().equals(ObjectCleaner.CLEANER_THREAD_NAME)) {
+                cleanerThread = thread;
+                break;
+            }
+        }
+        assertNotNull(cleanerThread);
+        assertTrue(cleanerThread.isDaemon());
+    }
 }