From 1bec49d0736bfd9baad39d7fcdf80a1daac73386 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 22 Jun 2016 16:01:49 -0400 Subject: [PATCH] Remove global BindingToNormalizedNodeCodec instance The BindingToNormalizedNodeCodec was made a global static instance for backwards compatibility for CSS users that inject the binding-dom-mapping-service identity which defines the provided service as the concrete BindingToNormalizedNodeCodec class instead of an interface. Therefore the global static instance was created via blueprint and advertised via its interfaces and was obtained via the static reference by the RuntimeMappingModule for use by CSS users. The RuntimeMappingModule must return an instance of BindingToNormalizedNodeCodec in order to provide the binding-dom-mapping-service so obtaining the blueprint advertised OSGi service via its interfaces and casting to BindingToNormalizedNodeCodec failed b/c Aries creates a service proxy which loses the fact that it's a BindingToNormalizedNodeCodec instance. However the global BindingToNormalizedNodeCodec instance is not clean and is problematic for supporting blueprint container restarts. Aries supports concrete class proxies so I added an additional service export for the BindingToNormalizedNodeCodec class in the binding-broker blueprint XML. In addition, the blueprint XML now calls a new method, "newInstance", on the BindingToNormalizedNodeCodecFactory to create a new instance and calls a new method, "registerInstance", to register it with the SchemaService. The returned ListenerRegistration instance is put into a bean so it can be closed on destroy. The RuntimeMappingModule now obtains the BindingToNormalizedNodeCodec instance from the OSGi registry as the other blueprint-bridged CSS modules do. This eliminates the need for the global instance. Change-Id: I969ad5470967a81b37078393701c69d1898086cd Signed-off-by: Tom Pantelis --- .../BindingToNormalizedNodeCodecFactory.java | 56 +++++++++++-------- .../opendaylight/blueprint/binding-broker.xml | 21 ++++++- .../binding/impl/RuntimeMappingModule.java | 45 +++++---------- 3 files changed, 65 insertions(+), 57 deletions(-) diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodecFactory.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodecFactory.java index 180f7b8f45..bb123a15f1 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodecFactory.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodecFactory.java @@ -7,51 +7,59 @@ */ package org.opendaylight.controller.md.sal.binding.impl; -import java.util.concurrent.atomic.AtomicBoolean; import org.opendaylight.controller.sal.binding.codegen.impl.SingletonHolder; import org.opendaylight.controller.sal.core.api.model.SchemaService; import org.opendaylight.yangtools.binding.data.codec.gen.impl.StreamWriterGenerator; import org.opendaylight.yangtools.binding.data.codec.impl.BindingNormalizedNodeCodecRegistry; +import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.sal.binding.generator.api.ClassLoadingStrategy; +import org.opendaylight.yangtools.yang.model.api.SchemaContextListener; /** - * Factory class for creating and initializing the global BindingToNormalizedNodeCodec instance. + * Factory class for creating and initializing the BindingToNormalizedNodeCodec instances. * * @author Thomas Pantelis */ public class BindingToNormalizedNodeCodecFactory { - private static final AtomicBoolean INSTANCE_CREATED = new AtomicBoolean(); - private static volatile BindingToNormalizedNodeCodec instance; - /** - * Returns the global BindingToNormalizedNodeCodec instance, creating if necessary. The returned instance - * is registered with tthe SchemaService as a SchemaContextListener. + * This method is deprecated in favor of newInstance/registerInstance. * * @param classLoadingStrategy * @param schemaService - * @return the BindingToNormalizedNodeCodec instance + * @return BindingToNormalizedNodeCodec instance */ + @Deprecated public static BindingToNormalizedNodeCodec getOrCreateInstance(ClassLoadingStrategy classLoadingStrategy, - SchemaService schemaService) { - if(!INSTANCE_CREATED.compareAndSet(false, true)) { - return instance; - } - + SchemaService schemaService) { BindingNormalizedNodeCodecRegistry codecRegistry = new BindingNormalizedNodeCodecRegistry( StreamWriterGenerator.create(SingletonHolder.JAVASSIST)); - BindingToNormalizedNodeCodec localInstance = new BindingToNormalizedNodeCodec( - classLoadingStrategy, codecRegistry, true); - - schemaService.registerSchemaContextListener(localInstance); - - // Publish the BindingToNormalizedNodeCodec instance after we've registered it as a - // SchemaContextListener to avoid a race condition by publishing it too early when it isn't - // fully initialized. - instance = localInstance; + BindingToNormalizedNodeCodec instance = new BindingToNormalizedNodeCodec( + classLoadingStrategy, codecRegistry, true); + schemaService.registerSchemaContextListener(instance); return instance; } - public static BindingToNormalizedNodeCodec getInstance() { - return instance; + /** + * Creates a new BindingToNormalizedNodeCodec instance. + * + * @param classLoadingStrategy + * @return the BindingToNormalizedNodeCodec instance + */ + public static BindingToNormalizedNodeCodec newInstance(ClassLoadingStrategy classLoadingStrategy) { + BindingNormalizedNodeCodecRegistry codecRegistry = new BindingNormalizedNodeCodecRegistry( + StreamWriterGenerator.create(SingletonHolder.JAVASSIST)); + return new BindingToNormalizedNodeCodec(classLoadingStrategy, codecRegistry, true); + } + + /** + * Registers the given instance with the SchemaService as a SchemaContextListener. + * + * @param instance the BindingToNormalizedNodeCodec instance + * @param schemaService the SchemaService. + * @return the ListenerRegistration + */ + public static ListenerRegistration registerInstance(BindingToNormalizedNodeCodec instance, + SchemaService schemaService) { + return schemaService.registerSchemaContextListener(instance); } } diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/resources/org/opendaylight/blueprint/binding-broker.xml b/opendaylight/md-sal/sal-binding-broker/src/main/resources/org/opendaylight/blueprint/binding-broker.xml index a8830169d2..79d0868d0d 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/resources/org/opendaylight/blueprint/binding-broker.xml +++ b/opendaylight/md-sal/sal-binding-broker/src/main/resources/org/opendaylight/blueprint/binding-broker.xml @@ -8,8 +8,14 @@ + factory-method="newInstance"> + + + + + @@ -22,6 +28,19 @@ + + + + + + + + + + diff --git a/opendaylight/md-sal/sal-binding-config/src/main/java/org/opendaylight/controller/config/yang/md/sal/binding/impl/RuntimeMappingModule.java b/opendaylight/md-sal/sal-binding-config/src/main/java/org/opendaylight/controller/config/yang/md/sal/binding/impl/RuntimeMappingModule.java index cc44cd84fb..89cdb06d19 100644 --- a/opendaylight/md-sal/sal-binding-config/src/main/java/org/opendaylight/controller/config/yang/md/sal/binding/impl/RuntimeMappingModule.java +++ b/opendaylight/md-sal/sal-binding-config/src/main/java/org/opendaylight/controller/config/yang/md/sal/binding/impl/RuntimeMappingModule.java @@ -8,11 +8,8 @@ package org.opendaylight.controller.config.yang.md.sal.binding.impl; import com.google.common.base.Preconditions; -import com.google.common.base.Stopwatch; -import com.google.common.util.concurrent.Uninterruptibles; -import java.util.concurrent.TimeUnit; +import org.opendaylight.controller.config.api.osgi.WaitingServiceTracker; import org.opendaylight.controller.md.sal.binding.impl.BindingToNormalizedNodeCodec; -import org.opendaylight.controller.md.sal.binding.impl.BindingToNormalizedNodeCodecFactory; import org.osgi.framework.BundleContext; /** @@ -20,8 +17,6 @@ import org.osgi.framework.BundleContext; */ @Deprecated public final class RuntimeMappingModule extends AbstractRuntimeMappingModule { - private static final long WAIT_IN_MINUTES = 5; - private BundleContext bundleContext; public RuntimeMappingModule(final org.opendaylight.controller.config.api.ModuleIdentifier identifier, @@ -48,32 +43,18 @@ public final class RuntimeMappingModule extends AbstractRuntimeMappingModule { } @Override - public java.lang.AutoCloseable createInstance() { - // This is kind of ugly - you might cringe (you've been warned). The BindingToNormalizedNodeCodec - // instance is advertised via blueprint so ideally we'd obtain it from the OSGi service registry. - // The config yang service identity declares the concrete BindingToNormalizedNodeCodec class - // and not an interface as the java-class so we must return a BindingToNormalizedNodeCodec instance. - // However we can't cast the instance obtained from the service registry to - // BindingToNormalizedNodeCodec b/c Aries may register a proxy if there are interceptors defined. - // By default karaf ships with the org.apache.aries.quiesce.api bundle which automatically adds - // an interceptor that adds stat tracking for service method calls. While this can be disabled, we - // shouldn't rely on it. - // - // Therefore we store a static instance in the BindingToNormalizedNodeCodecFactory which is created - // by blueprint via newInstance. We obtain the static instance here and busy wait if not yet available. - - Stopwatch sw = Stopwatch.createStarted(); - while(sw.elapsed(TimeUnit.MINUTES) <= WAIT_IN_MINUTES) { - BindingToNormalizedNodeCodec instance = BindingToNormalizedNodeCodecFactory.getInstance(); - if(instance != null) { - return instance; - } - - Uninterruptibles.sleepUninterruptibly(50, TimeUnit.MILLISECONDS); - } - - throw new IllegalStateException("Could not obtain the BindingToNormalizedNodeCodec instance after " + - WAIT_IN_MINUTES + " minutes."); + public AutoCloseable createInstance() { + // We need to return the concrete BindingToNormalizedNodeCodec instance for backwards compatibility + // for CSS users that inject the binding-dom-mapping-service. + final WaitingServiceTracker tracker = + WaitingServiceTracker.create(BindingToNormalizedNodeCodec.class, bundleContext); + final BindingToNormalizedNodeCodec service = tracker.waitForService(WaitingServiceTracker.FIVE_MINUTES); + + // Ideally we would close the ServiceTracker via the returned AutoCloseable but then we'd have to + // proxy the BindingToNormalizedNodeCodec instance which is problematic. It's OK to close the + // ServiceTracker here. + tracker.close(); + return service; } public void setBundleContext(final BundleContext bundleContext) { -- 2.36.6