Merge pull request #121 from bugsnag/fix-application-hang

[PLAT-2110] Use deamon threads to start flushing sessions

Author: fractalwrench

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## 3.4.2 (2018-11-28)
+
+ * Prevent application hangs due to session flushing
+[#121](https://github.com/bugsnag/bugsnag-java/pull/121)
+
 ## 3.4.1
 
 (Skipped, duplicate of 3.4.0)

bugsnag/src/main/java/com/bugsnag/Bugsnag.java

@@ -3,6 +3,7 @@
 import com.bugsnag.callbacks.Callback;
 import com.bugsnag.delivery.Delivery;
 import com.bugsnag.delivery.HttpDelivery;
+import com.bugsnag.util.DaemonThreadFactory;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -12,6 +13,9 @@
 import java.util.Collections;
 import java.util.Date;
 import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.RejectedExecutionHandler;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.ThreadPoolExecutor;
@@ -23,8 +27,17 @@ public class Bugsnag {
     private static final int SESSION_TRACKING_PERIOD_MS = 60000;
     private static final int CORE_POOL_SIZE = 1;
 
+    // Create an executor service which keeps idle threads alive for a maximum of SHUTDOWN_TIMEOUT.
+    // This should avoid blocking an application that doesn't call shutdown from exiting.
+    private ExecutorService sessionFlusherService =
+            new ThreadPoolExecutor(0, 1,
+                    SHUTDOWN_TIMEOUT_MS, TimeUnit.MILLISECONDS,
+                    new LinkedBlockingQueue<Runnable>());
+
     private ScheduledThreadPoolExecutor sessionExecutorService =
-            new ScheduledThreadPoolExecutor(CORE_POOL_SIZE, new RejectedExecutionHandler() {
+            new ScheduledThreadPoolExecutor(CORE_POOL_SIZE,
+                    new DaemonThreadFactory(),
+                    new RejectedExecutionHandler() {
                 @Override
                 public void rejectedExecution(Runnable runnable, ThreadPoolExecutor executor) {
                     LOGGER.error("Rejected execution for sessionExecutorService");
@@ -81,7 +94,14 @@ private void scheduleSessionFlushes() {
         sessionExecutorService.scheduleAtFixedRate(new Runnable() {
             @Override
             public void run() {
-                sessionTracker.flushSessions(new Date());
+                // Use a different thread which is not a daemon thread
+                // to actually flush the sessions
+                sessionFlusherService.submit(new Runnable() {
+                    @Override
+                    public void run() {
+                        sessionTracker.flushSessions(new Date());
+                    }
+                });
             }
         }, SESSION_TRACKING_PERIOD_MS, SESSION_TRACKING_PERIOD_MS, TimeUnit.MILLISECONDS);
     }

bugsnag/src/main/java/com/bugsnag/Notifier.java

@@ -5,7 +5,7 @@
 class Notifier {
 
     private static final String NOTIFIER_NAME = "Bugsnag Java";
-    private static final String NOTIFIER_VERSION = "3.4.0";
+    private static final String NOTIFIER_VERSION = "3.4.2";
     private static final String NOTIFIER_URL = "https://github.com/bugsnag/bugsnag-java";
 
     private String notifierName = NOTIFIER_NAME;

bugsnag/src/main/java/com/bugsnag/util/DaemonThreadFactory.java

@@ -0,0 +1,31 @@
+package com.bugsnag.util;
+
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+
+/**
+ * Wraps {@link Executors#defaultThreadFactory()} to return daemon threads
+ * This is to prevent applications from hanging waiting for the sessions scheduled task
+ * because daemon threads will be terminated on application shutdown
+ */
+public class DaemonThreadFactory implements ThreadFactory {
+    private final ThreadFactory defaultThreadFactory;
+
+    /**
+     * Constructor
+     */
+    public DaemonThreadFactory() {
+        defaultThreadFactory = Executors.defaultThreadFactory();
+    }
+
+    @Override
+    public Thread newThread(Runnable runner) {
+        Thread daemonThread = defaultThreadFactory.newThread(runner);
+
+        // Set the threads to daemon to allow the app to shutdown properly
+        if (!daemonThread.isDaemon()) {
+            daemonThread.setDaemon(true);
+        }
+        return daemonThread;
+    }
+}

bugsnag/src/test/java/com/bugsnag/DaemonThreadFactoryTest.java

@@ -0,0 +1,33 @@
+package com.bugsnag;
+
+import static org.junit.Assert.assertTrue;
+
+import com.bugsnag.util.DaemonThreadFactory;
+
+import org.junit.Before;
+import org.junit.Test;
+
+
+/**
+ * Tests for the Daemon thread factory internal logic
+ */
+public class DaemonThreadFactoryTest {
+
+    private DaemonThreadFactory daemonThreadFactory;
+
+    /**
+     * Create the daemonThreadFactory before the tests
+     */
+    @Before
+    public void createFactory() {
+        daemonThreadFactory = new DaemonThreadFactory();
+    }
+
+    @Test
+    public void testDaemonThreadFactory() {
+        Thread testThread = daemonThreadFactory.newThread(null);
+
+        // Check that the thread is as expected
+        assertTrue(testThread.isDaemon());
+    }
+}

gradle.properties

@@ -1,4 +1,4 @@
-version=3.4.1
+version=3.4.2
 group=com.bugsnag
 
 # Default properties