From 0b02d94edd5487d702a7bb790219a5c519d7f6b5 Mon Sep 17 00:00:00 2001
From: Robert Varga <rovarga@cisco.com>
Date: Wed, 9 Dec 2015 15:13:35 +0100
Subject: [PATCH] BUG-4514: do not retain old internal info

Retaining this information past second phase commit leads to it being
retained via dependencyResolver. Thus a series of reconfigurations will
invariably retain the complete history, which is completely wasteful and
constitutes a memory leak (with GC chains of ~1800 hops observed).

Change-Id: Id67b8813a1d55b36f0b55a1c96099b906bf313ad
Signed-off-by: Robert Varga <rovarga@cisco.com>
---
 .../config/manager/impl/ConfigRegistryImpl.java     |  5 +++++
 .../ModuleInternalTransactionalInfo.java            | 13 +++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
index 39ea73be55..da6b83c86e 100644
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ConfigRegistryImpl.java
@@ -338,6 +338,11 @@ public class ConfigRegistryImpl implements AutoCloseable, ConfigRegistryImplMXBe
 
                 // close old module jmx registrator
                 oldInternalInfo.getModuleJMXRegistrator().close();
+
+                // We no longer need old internal info. Clear it out, so we do not create a serial leak evidenced
+                // by BUG-4514. The reason is that modules retain their resolver, which retains modules. If we retain
+                // the old module, we would have the complete reconfiguration history held in heap for no good reason.
+                entry.clearOldInternalInfo();
             } else {
                 // new instance:
                 // wrap in readable dynamic mbean
diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
index 210b6d2aa7..fc9b645ea2 100644
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/dependencyresolver/ModuleInternalTransactionalInfo.java
@@ -8,6 +8,7 @@
 package org.opendaylight.controller.config.manager.impl.dependencyresolver;
 
 import com.google.common.base.Preconditions;
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.config.api.ModuleIdentifier;
 import org.opendaylight.controller.config.manager.impl.ModuleInternalInfo;
@@ -22,11 +23,11 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
     private final ModuleIdentifier name;
     private final Module proxiedModule, realModule;
     private final ModuleFactory moduleFactory;
-    @Nullable
-    private final ModuleInternalInfo maybeOldInternalInfo;
+
     private final TransactionModuleJMXRegistration transactionModuleJMXRegistration;
     private final boolean isDefaultBean;
     private final BundleContext bundleContext;
+    @Nullable private ModuleInternalInfo maybeOldInternalInfo;
 
     public ModuleInternalTransactionalInfo(ModuleIdentifier name, Module proxiedModule,
                                            ModuleFactory moduleFactory,
@@ -69,11 +70,15 @@ public class ModuleInternalTransactionalInfo implements Identifiable<ModuleIdent
         return moduleFactory;
     }
 
-    @Nullable
-    public ModuleInternalInfo getOldInternalInfo() {
+    @Nonnull public ModuleInternalInfo getOldInternalInfo() {
         return Preconditions.checkNotNull(maybeOldInternalInfo);
     }
 
+    public void clearOldInternalInfo() {
+        Preconditions.checkState(maybeOldInternalInfo != null, "No old internal info present");
+        maybeOldInternalInfo = null;
+    }
+
     public TransactionModuleJMXRegistration getTransactionModuleJMXRegistration() {
         return transactionModuleJMXRegistration;
     }
-- 
2.36.6