Skip to content

Commit

Permalink
ObjectCleanerThread must be a deamon thread to ensure the JVM can alw…
Browse files Browse the repository at this point in the history
…ays 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 [netty#7617].
  • Loading branch information
normanmaurer committed Jan 26, 2018
1 parent f99a198 commit 1879433
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
12 changes: 6 additions & 6 deletions common/src/main/java/io/netty/util/internal/ObjectCleaner.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,11 +34,13 @@
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() {
Expand Down Expand Up @@ -116,9 +116,9 @@ public Void run() {
});
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();
}
}
Expand Down
24 changes: 24 additions & 0 deletions common/src/test/java/io/netty/util/internal/ObjectCleanerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
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 {

Expand Down Expand Up @@ -110,4 +112,26 @@ public void run() {
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());
}
}

0 comments on commit 1879433

Please sign in to comment.