Commits

Bryan Turner committed 05ca813

Fixed ExternalProcessImpl#shutdown() to use the ExecutorService awaitTermination method correctly

Comments (0)

Files changed (1)

src/main/java/com/atlassian/utils/process/ExternalProcessImpl.java

      * Attempts to shutdown the internal {@code ThreadPoolExecutor} which manages the I/O pumps for external processes.
      * To prevent memory leaks, web applications which use process-utils should always call this when they terminate.
      * <p/>
-     * On termination, an attempt is made to shutdown the thread pool gracefully. If that does not complete within one
-     * second, shutdown is forced. That is given another second to complete before the thread pool is abandoned.
+     * On termination, an attempt is made to shutdown the thread pool gracefully. If that does not complete within five
+     * seconds, shutdown is forced. That is given another five seconds to complete before the thread pool is abandoned.
      *
      * @since 1.5
      */
         if (POOL == null) {
             return;
         }
-
-        LOG.debug("Attempting to shutdown ThreadPoolExecutor");
+        LOG.debug("Attempting to shutdown pump executor service");
 
         POOL.shutdown();
         try {
-            POOL.awaitTermination(1, TimeUnit.SECONDS);
-        } catch (InterruptedException e) {
-            //The thread pool did not shutdown within the timeout. We can't wait forever, though.
-        } finally {
-            if (POOL.isTerminated()) {
-                LOG.debug("ThreadPoolExecutor shutdown gracefully");
+            if (POOL.awaitTermination(5, TimeUnit.SECONDS)) {
+                LOG.debug("Pump executor service has shutdown gracefully");
             } else {
-                LOG.warn("ThreadPoolExecutor did not shutdown within the timeout; forcing shutdown");
+                //The executor did not shutdown within the timeout. We can't wait forever, though, so issue a
+                //shutdownNow() and give it another second
+                LOG.warn("Pump executor service did not shutdown within the timeout; forcing shutdown");
 
                 POOL.shutdownNow();
-                try {
-                    POOL.awaitTermination(1, TimeUnit.SECONDS);
-                } catch (InterruptedException ie) {
-                    LOG.warn("ThreadPoolExecutor not shutdown; it will be abandoned");
+                if (POOL.awaitTermination(5, TimeUnit.SECONDS)) {
+                    //The forced shutdown has brought the executor down. Not ideal, but acceptable
+                    LOG.debug("Pump executor service has been forced to shutdown");
+                } else {
+                    //We can't delay execution indefinitely waiting, so log a warning. The JVM may not shut down
+                    //if this service does not stop (because it uses non-daemon threads), so this may be helpful
+                    //in debugging should that happen.
+                    LOG.warn("Pump executor service did not shutdown; it will be abandoned");
                 }
             }
+        } catch (InterruptedException e) {
+            LOG.warn("Interrupted while waiting for the pump executor service to shutdown; some worker threads may " +
+                    "still be running");
         }
     }
 }