From 3ae0855ef5d1837fb0c47c2471ced766d35aa5c4 Mon Sep 17 00:00:00 2001
From: Rsl1122 <djrapitops@hotmail.com>
Date: Sat, 15 Sep 2018 10:38:05 +0300
Subject: [PATCH] Named Plan ExecutorService pools, Fixed WebServer thread leak
 on reload

WebServer ThreadPoolExecutor was never shutdown, as it was assumed
HTTPServer.shutdown() would perform that.
In extreme cases 250 reloads could lead to a OutOfMemoryException due to
Heap size allocation for threads not being possible.

Change: Shut down ThreadPoolExecutor manually.
---
 .../plan/system/processing/Processing.java    |  5 ++--
 .../plan/system/webserver/WebServer.java      | 27 +++++++++++++++----
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/Plan/src/main/java/com/djrapitops/plan/system/processing/Processing.java b/Plan/src/main/java/com/djrapitops/plan/system/processing/Processing.java
index 35a039693..ce9e9df9a 100644
--- a/Plan/src/main/java/com/djrapitops/plan/system/processing/Processing.java
+++ b/Plan/src/main/java/com/djrapitops/plan/system/processing/Processing.java
@@ -9,6 +9,7 @@ import com.djrapitops.plan.system.locale.lang.PluginLang;
 import com.djrapitops.plugin.StaticHolder;
 import com.djrapitops.plugin.api.utility.log.Log;
 import com.djrapitops.plugin.utilities.Verify;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
 import java.util.List;
 import java.util.concurrent.*;
@@ -23,8 +24,8 @@ public class Processing implements SubSystem {
 
     public Processing(Supplier<Locale> locale) {
         this.locale = locale;
-        nonCriticalExecutor = Executors.newFixedThreadPool(6);
-        criticalExecutor = Executors.newFixedThreadPool(2);
+        nonCriticalExecutor = Executors.newFixedThreadPool(6, new ThreadFactoryBuilder().setNameFormat("Plan Non critical-pool-%d").build());
+        criticalExecutor = Executors.newFixedThreadPool(2, new ThreadFactoryBuilder().setNameFormat("Plan Critical-pool-%d").build());
         saveInstance(nonCriticalExecutor);
         saveInstance(criticalExecutor);
         saveInstance(this);
diff --git a/Plan/src/main/java/com/djrapitops/plan/system/webserver/WebServer.java b/Plan/src/main/java/com/djrapitops/plan/system/webserver/WebServer.java
index c8d90b4e3..d2b4a4a87 100644
--- a/Plan/src/main/java/com/djrapitops/plan/system/webserver/WebServer.java
+++ b/Plan/src/main/java/com/djrapitops/plan/system/webserver/WebServer.java
@@ -12,6 +12,7 @@ import com.djrapitops.plugin.StaticHolder;
 import com.djrapitops.plugin.api.Check;
 import com.djrapitops.plugin.api.utility.log.Log;
 import com.djrapitops.plugin.utilities.Verify;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.sun.net.httpserver.HttpServer;
 import com.sun.net.httpserver.HttpsConfigurator;
 import com.sun.net.httpserver.HttpsParameters;
@@ -27,9 +28,7 @@ import java.nio.file.Paths;
 import java.security.*;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
-import java.util.concurrent.ArrayBlockingQueue;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.*;
 import java.util.function.Supplier;
 
 /**
@@ -108,7 +107,11 @@ public class WebServer implements SubSystem {
             }
             server.createContext("/", requestHandler);
 
-            server.setExecutor(new ThreadPoolExecutor(4, 8, 30, TimeUnit.SECONDS, new ArrayBlockingQueue<>(100)));
+            ExecutorService executor = new ThreadPoolExecutor(
+                    4, 8, 30, TimeUnit.SECONDS, new ArrayBlockingQueue<>(100),
+                    new ThreadFactoryBuilder().setNameFormat("Plan WebServer Thread-%d").build()
+            );
+            server.setExecutor(executor);
             server.start();
 
             enabled = true;
@@ -199,12 +202,26 @@ public class WebServer implements SubSystem {
     @Override
     public void disable() {
         if (server != null) {
+            shutdown();
             Log.info(locale.get().getString(PluginLang.DISABLED_WEB_SERVER));
-            server.stop(0);
         }
         enabled = false;
     }
 
+    private void shutdown() {
+        server.stop(0);
+        Executor executor = server.getExecutor();
+        if (executor instanceof ExecutorService) {
+            ExecutorService service = (ExecutorService) executor;
+            service.shutdown();
+            try {
+                service.awaitTermination(5, TimeUnit.SECONDS);
+            } catch (InterruptedException timeoutExceededEx) {
+                service.shutdownNow();
+            }
+        }
+    }
+
     public String getProtocol() {
         return usingHttps ? "https" : "http";
     }