From: Robert Varga Date: Wed, 2 Dec 2015 14:20:22 +0000 (+0100) Subject: BUG-4514: clear oldModule/oldInstance when no longer needed X-Git-Tag: release/lithium-sr4~37 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=9708d3438a661c6065f81bd44d00cc8a7242ec6f;p=controller.git 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 (cherry picked from commit 7ff9016372201b5a14fd5b6f2e1faba1bc3760f3) --- 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 772bacce89..303fc2a072 100644 --- 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 @@ -13,11 +13,12 @@ public abstract class AbstractModule> 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. @@ -25,7 +26,7 @@ public abstract class AbstractModule> 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); } @@ -37,7 +38,7 @@ public abstract class AbstractModule> 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; @@ -57,15 +58,15 @@ public abstract class AbstractModule> 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); } } @@ -75,7 +76,12 @@ public abstract class AbstractModule> 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; } @@ -85,7 +91,7 @@ public abstract class AbstractModule> 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; @@ -107,7 +113,7 @@ public abstract class AbstractModule> 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; }