Remove global BindingToNormalizedNodeCodec instance 43/40743/7
authorTom Pantelis <tpanteli@brocade.com>
Wed, 22 Jun 2016 20:01:49 +0000 (16:01 -0400)
committerTom Pantelis <tpanteli@brocade.com>
Thu, 21 Jul 2016 18:47:47 +0000 (18:47 +0000)
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 <tpanteli@brocade.com>
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodecFactory.java
opendaylight/md-sal/sal-binding-broker/src/main/resources/org/opendaylight/blueprint/binding-broker.xml
opendaylight/md-sal/sal-binding-config/src/main/java/org/opendaylight/controller/config/yang/md/sal/binding/impl/RuntimeMappingModule.java

index 180f7b8..bb123a1 100644 (file)
@@ -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<SchemaContextListener> registerInstance(BindingToNormalizedNodeCodec instance,
+            SchemaService schemaService) {
+        return schemaService.registerSchemaContextListener(instance);
     }
 }
index a883016..79d0868 100644 (file)
@@ -8,8 +8,14 @@
   <reference id="schemaService" interface="org.opendaylight.controller.sal.core.api.model.SchemaService" />
 
   <bean id="mappingCodec" class="org.opendaylight.controller.md.sal.binding.impl.BindingToNormalizedNodeCodecFactory"
-         factory-method="getOrCreateInstance">
+         factory-method="newInstance">
     <argument ref="classLoadingStrategy"/>
+  </bean>
+
+  <!-- Register the BindingToNormalizedNodeCodec with the SchemaService as a SchemaContextListener -->
+  <bean id="mappingCodecReg" class="org.opendaylight.controller.md.sal.binding.impl.BindingToNormalizedNodeCodecFactory"
+         factory-method="registerInstance" destroy-method="close">
+    <argument ref="mappingCodec"/>
     <argument ref="schemaService"/>
   </bean>
 
     </interfaces>
   </service>
 
+  <!-- We also register the BindingToNormalizedNodeCodec with its actual class name for
+       backwards compatibility for CSS users that inject the binding-dom-mapping-service -->
+  <service ref="mappingCodec" interface="org.opendaylight.controller.md.sal.binding.impl.BindingToNormalizedNodeCodec"
+        odl:type="default">
+    <!-- Set the appropriate service properties so the corresponding CSS module is restarted if this
+         blueprint container is restarted -->
+    <service-properties>
+      <entry key="config-module-namespace" value="urn:opendaylight:params:xml:ns:yang:controller:md:sal:binding:impl"/>
+      <entry key="config-module-name" value="runtime-generated-mapping"/>
+      <entry key="config-instance-name" value="runtime-mapping-singleton"/>
+    </service-properties>
+  </service>
+
   <!-- Binding RPC Registry Service -->
 
   <reference id="domRpcService" interface="org.opendaylight.controller.md.sal.dom.api.DOMRpcService"/>
index cc44cd8..89cdb06 100644 (file)
@@ -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<BindingToNormalizedNodeCodec> 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) {