Bugfix/fix resource leak in tests (#456)

DC2-DanielKrueger · web-flow · commit 25f598cc06d3 · 2024-06-13T11:13:53.000+02:00
* fix resource leak &gt; fix duplicate instatiation of shutdown hooks

* fix resource leak &gt; add shutdown hooks to close/clear resources

* fix resource leak &gt; add shutdown hook for the ThreadPool of the MessageForwarderImple
diff --git a/hivemq-edge/src/main/java/com/hivemq/bootstrap/HiveMQExceptionHandlerBootstrap.java b/hivemq-edge/src/main/java/com/hivemq/bootstrap/HiveMQExceptionHandlerBootstrap.java
@@ -56,6 +56,8 @@ static void handleUncaughtException(final Thread t, final Throwable e) {
             if (showException) {
                 log.error("Cause: ", e);
             }
+        } else if (rootCause instanceof ThreadDeath) {
+           // NOOP: this happens when tests stop the remaining threads via thread.stop();
         } else {
             log.error("Uncaught Error:", e);
         }
diff --git a/hivemq-edge/src/main/java/com/hivemq/bridge/MessageForwarderImpl.java b/hivemq-edge/src/main/java/com/hivemq/bridge/MessageForwarderImpl.java
@@ -21,6 +21,8 @@
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
+import com.hivemq.common.shutdown.HiveMQShutdownHook;
+import com.hivemq.common.shutdown.ShutdownHooks;
 import com.hivemq.configuration.HivemqId;
 import com.hivemq.configuration.service.InternalConfigurations;
 import com.hivemq.extension.sdk.api.annotations.NotNull;
@@ -77,11 +79,23 @@ public MessageForwarderImpl(
             final @NotNull LocalTopicTree topicTree,
             final @NotNull HivemqId hivemqId,
             final @NotNull Lazy<ClientQueuePersistence> queuePersistence,
-            final @NotNull SingleWriterService singleWriterService) {
+            final @NotNull SingleWriterService singleWriterService,
+            final @NotNull ShutdownHooks shutdownHooks) {
         this.topicTree = topicTree;
         this.hivemqId = hivemqId;
         this.queuePersistence = queuePersistence;
         this.singleWriterService = singleWriterService;
+        shutdownHooks.add(new HiveMQShutdownHook() {
+            @Override
+            public @NotNull String name() {
+                return "MessageForwarder-Shutdown";
+            }
+
+            @Override
+            public void run() {
+                executorService.shutdown();
+            }
+        });
     }
 
     @Override
diff --git a/hivemq-edge/src/main/java/com/hivemq/common/shutdown/ShutdownHooks.java b/hivemq-edge/src/main/java/com/hivemq/common/shutdown/ShutdownHooks.java
@@ -24,8 +24,6 @@
 import org.slf4j.LoggerFactory;
 import org.slf4j.MarkerFactory;
 
-import javax.inject.Inject;
-import javax.inject.Singleton;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -41,15 +39,13 @@
  *
  * @author Dominik Obermaier
  */
-@Singleton
 public class ShutdownHooks {
 
     private static final Logger log = LoggerFactory.getLogger(ShutdownHooks.class);
 
     private final @NotNull AtomicBoolean shuttingDown;
     private final @NotNull Multimap</* Priority */Integer, HiveMQShutdownHook> synchronousHooks;
 
-    @Inject
     public ShutdownHooks() {
         shuttingDown = new AtomicBoolean(false);
 
diff --git a/hivemq-edge/src/main/java/com/hivemq/edge/modules/ModuleLoader.java b/hivemq-edge/src/main/java/com/hivemq/edge/modules/ModuleLoader.java
@@ -25,7 +25,6 @@
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
-import javax.inject.Singleton;
 import java.io.File;
 import java.io.IOException;
 import java.net.URL;
@@ -34,15 +33,13 @@
 import java.util.List;
 import java.util.Set;
 
-@Singleton
 public class ModuleLoader {
     private static final Logger log = LoggerFactory.getLogger(ModuleLoader.class);
 
     private final @NotNull SystemInformation systemInformation;
     protected final @NotNull Set<EdgeModule> modules = new HashSet<>();
     private final @NotNull ClassServiceLoader classServiceLoader = new ClassServiceLoader();
 
-    @Inject
     public ModuleLoader(final @NotNull SystemInformation systemInformation) {
         this.systemInformation = systemInformation;
     }
@@ -93,7 +90,7 @@ protected void discoverWorkspaceModule(final File dir, final ClassLoader parentC
                     }
                     final IsolatedModuleClassloader isolatedClassloader =
                             new IsolatedModuleClassloader(urls.toArray(new URL[0]), parentClassloader);
-                    modules.add(new EdgeModule(files[i], isolatedClassloader));
+                    modules.add(new EdgeModule(files[i], isolatedClassloader, true));
                 } catch (final IOException ioException) {
                     log.warn("Exception with reason {} while reading module file {}",
                             ioException.getMessage(),
@@ -115,7 +112,7 @@ protected void loadFromModulesDirectory(final ClassLoader parentClassloader) {
                         log.debug("Found module jar in modules lib {}.", lib.getAbsolutePath());
                         final IsolatedModuleClassloader isolatedClassloader =
                                 new IsolatedModuleClassloader(new URL[]{lib.toURI().toURL()}, parentClassloader);
-                        modules.add(new EdgeModule(lib, isolatedClassloader));
+                        modules.add(new EdgeModule(lib, isolatedClassloader, true));
                     } else {
                         log.debug("Ignoring non jar file in module folder {}.", lib.getAbsolutePath());
                     }
@@ -154,15 +151,19 @@ protected void loadFromModulesDirectory(final ClassLoader parentClassloader) {
         return modules;
     }
 
+    public void clear() {
+        modules.clear();
+    }
+
     public static class EdgeModule {
 
         private File root;
         private final @NotNull ClassLoader classloader;
 
-        public EdgeModule(final @NotNull File root, final @NotNull ClassLoader classloader) {
+        public EdgeModule(final @NotNull File root, final @NotNull ClassLoader classloader, final boolean registerStaticResources) {
             this.classloader = classloader;
             this.root = root;
-            if (classloader != null) {
+            if (registerStaticResources) {
                 AlternativeClassloadingStaticFileHandler.addClassLoader(classloader);
             }
         }
diff --git a/hivemq-edge/src/main/java/com/hivemq/http/JaxrsHttpServer.java b/hivemq-edge/src/main/java/com/hivemq/http/JaxrsHttpServer.java
@@ -62,6 +62,7 @@ public class JaxrsHttpServer {
     static final String MAX_RESP_TIME = "sun.net.httpserver.maxRspTime";
     private static final Logger logger = LoggerFactory.getLogger(JaxrsHttpServer.class);
     private final @NotNull List<HttpServer> httpServers = new ArrayList<>();
+    private final @NotNull ShutdownHooks shutdownHooks;
     private final @NotNull List<JaxrsHttpServerConfiguration> configs;
     private final @Nullable ResourceConfig resourceConfig;
     private @Nullable JaxrsObjectMapperProvider objectMapperProvider;
@@ -72,6 +73,7 @@ public JaxrsHttpServer(
             final @NotNull ShutdownHooks shutdownHooks,
             final @NotNull List<JaxrsHttpServerConfiguration> configs,
             final @Nullable ResourceConfig resourceConfig) {
+        this.shutdownHooks = shutdownHooks;
         this.configs = configs;
         this.resourceConfig = resourceConfig;
         shutdownHooks.add(new Shutdown());
@@ -181,7 +183,8 @@ public void startServer() {
                                 httpServer);
                         registerContext("/module/images",
                                 new AlternativeClassloadingStaticFileHandler(objectMapperProvider.getMapper(),
-                                        "httpd/images"),
+                                        "httpd/images",
+                                        shutdownHooks),
                                 httpServer);
 
 
diff --git a/hivemq-edge/src/main/java/com/hivemq/http/handlers/AlternativeClassloadingStaticFileHandler.java b/hivemq-edge/src/main/java/com/hivemq/http/handlers/AlternativeClassloadingStaticFileHandler.java
@@ -16,10 +16,10 @@
 package com.hivemq.http.handlers;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.hivemq.http.core.HttpUtils;
-import com.hivemq.http.core.IHttpRequestResponse;
+import com.hivemq.common.shutdown.HiveMQShutdownHook;
+import com.hivemq.common.shutdown.ShutdownHooks;
+import com.hivemq.extension.sdk.api.annotations.NotNull;
 
-import java.io.IOException;
 import java.io.InputStream;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -31,20 +31,24 @@
  * allowing modules to contribute arbitrary resources
  */
 public class AlternativeClassloadingStaticFileHandler extends StaticFileHandler {
-    protected static Set<ClassLoader> classLoaders;
+    protected final static @NotNull Set<ClassLoader> classLoaders = new HashSet<>();;
 
-    public AlternativeClassloadingStaticFileHandler(ObjectMapper mapper, String resourceRoot) {
+    public AlternativeClassloadingStaticFileHandler(ObjectMapper mapper, String resourceRoot, ShutdownHooks shutdownHooks) {
         super(mapper, resourceRoot);
+        shutdownHooks.add(new HiveMQShutdownHook() {
+            @Override
+            public @NotNull String name() {
+                return "AlternativeClassloadingStaticFileHandler - Shutdown";
+            }
+
+            @Override
+            public void run() {
+                classLoaders.clear();
+            }
+        });
     }
 
     public static void addClassLoader(ClassLoader classLoader){
-        if(classLoaders == null){
-            synchronized (AlternativeClassloadingStaticFileHandler.class){
-                if(classLoaders == null){
-                    classLoaders = new HashSet<>();
-                }
-            }
-        }
         classLoaders.add(classLoader);
     }
 

Original file line number	Diff line number	Diff line change
`@@ -56,6 +56,8 @@ static void handleUncaughtException(final Thread t, final Throwable e) {`
`56`	`56`	`if (showException) {`
`57`	`57`	`log.error("Cause: ", e);`
`58`	`58`	`}`
	`59`	`+ } else if (rootCause instanceof ThreadDeath) {`
	`60`	`+ // NOOP: this happens when tests stop the remaining threads via thread.stop();`
`59`	`61`	`} else {`
`60`	`62`	`log.error("Uncaught Error:", e);`
`61`	`63`	`}`