BUG-4514: clear oldModule/oldInstance when no longer needed

When we acquire a new instance we no longer need the old module nor
instance. Failing to clear these references leads to slow memory leak,
as we retain the complete history of modules as transactions are being
made.

Change-Id: I42899176d335e5f6ac66ecb1dfe080c4dd14ab2a
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java
index b0fda1eacf936a392718371238399f15a7666e6d..e60a047379ee73b9399807f232a08592123d0d5c 100644 (file)
--- a/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java
+++ b/opendaylight/config/config-api/src/main/java/org/opendaylight/controller/config/spi/AbstractModule.java
@@ -21,11 +21,12 @@ public abstract class AbstractModule<M extends AbstractModule<M>> implements org
 
     private static final Logger LOG = LoggerFactory.getLogger(AbstractModule.class);
 
-    private final M oldModule;
-    private final AutoCloseable oldInstance;
+    protected final DependencyResolver dependencyResolver;
     protected final ModuleIdentifier identifier;
+
+    private AutoCloseable oldInstance;
+    private M oldModule;
     private AutoCloseable instance;
-    protected final DependencyResolver dependencyResolver;
 
     /**
      * Called when module is configured.
@@ -33,7 +34,7 @@ public abstract class AbstractModule<M extends AbstractModule<M>> implements org
      * @param identifier id of current instance.
      * @param dependencyResolver resolver used in dependency injection and validation.
      */
-    public AbstractModule(ModuleIdentifier identifier, DependencyResolver dependencyResolver) {
+    public AbstractModule(final ModuleIdentifier identifier, final DependencyResolver dependencyResolver) {
         this(identifier, dependencyResolver, null, null);
     }
 
@@ -45,7 +46,7 @@ public abstract class AbstractModule<M extends AbstractModule<M>> implements org
      * @param oldModule old instance of module that is being reconfigred(replaced) by current instance. The old instance can be examined for reuse.
      * @param oldInstance old instance wrapped by the old module. This is the resource that is actually being reused if possible or closed otherwise.
      */
-    public AbstractModule(ModuleIdentifier identifier, DependencyResolver dependencyResolver, M oldModule, AutoCloseable oldInstance) {
+    public AbstractModule(final ModuleIdentifier identifier, final DependencyResolver dependencyResolver, final M oldModule, final AutoCloseable oldInstance) {
         this.identifier = identifier;
         this.dependencyResolver = dependencyResolver;
         this.oldModule = oldModule;
@@ -65,15 +66,15 @@ public abstract class AbstractModule<M extends AbstractModule<M>> implements org
      */
     @Override
     public final AutoCloseable getInstance() {
-        if(instance==null) {
-            if(oldInstance!=null && canReuseInstance(oldModule)) {
+        if (instance == null) {
+            if (oldInstance != null && canReuseInstance(oldModule)) {
                 resolveDependencies();
                 instance = reuseInstance(oldInstance);
             } else {
-                if(oldInstance!=null) {
+                if (oldInstance != null) {
                     try {
                         oldInstance.close();
-                    } catch(Exception e) {
+                    } catch (Exception e) {
                         LOG.error("An error occurred while closing old instance {} for module {}", oldInstance, getIdentifier(), e);
                     }
                 }
@@ -83,7 +84,12 @@ public abstract class AbstractModule<M extends AbstractModule<M>> implements org
                     throw new IllegalStateException("Error in createInstance - null is not allowed as return value. Module: " + getIdentifier());
                 }
             }
+
+            // Prevent serial memory leak: clear these references as we will not use them again.
+            oldInstance = null;
+            oldModule = null;
         }
+
         return instance;
     }
 
@@ -93,7 +99,7 @@ public abstract class AbstractModule<M extends AbstractModule<M>> implements org
     protected abstract AutoCloseable createInstance();
 
     @Override
-    public final boolean canReuse(Module oldModule) {
+    public final boolean canReuse(final Module oldModule) {
         // Just cast into a specific instance
         // TODO unify this method with canReuseInstance (required Module interface to be generic which requires quite a lot of changes)
         return getClass().isInstance(oldModule) ? canReuseInstance((M) oldModule) : false;
@@ -115,7 +121,7 @@ public abstract class AbstractModule<M extends AbstractModule<M>> implements org
      * @param oldInstance old instance of a class wrapped by the module
      * @return reused instance
      */
-    protected AutoCloseable reuseInstance(AutoCloseable oldInstance) {
+    protected AutoCloseable reuseInstance(final AutoCloseable oldInstance) {
         // implement if instance reuse should be supported. Override canReuseInstance to change the criteria.
         return oldInstance;
     }