Lazily create schema context in GlobalBundleScanning*

On GlobalBundleScanningSchemaServiceImpl startup, it calls
tryToUpdateSchemaContext when bundleTracker.open() completes.
This parses and creates a schema context and notifies listeners
of the updated schema context. However we don't have to call
tryToUpdateSchemaContext in start() unless there actually are
listeners registered as the purpose of tryToUpdateSchemaContext
is to update listeners. So made this change. It's not likely there
would be any listeners at that point anway.

I also changed the synchronization so as not to synchronize on 'this'
or the class instance as both are unsafe (users could sync on the
instance causing unwanted contention, either innocently or maliciously).

Change-Id: I435358b0851671b7fbfdc9784577c91ff20556df
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>

diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/GlobalBundleScanningSchemaServiceImpl.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/GlobalBundleScanningSchemaServiceImpl.java
index ac220a830d2694274a53e3efb03d465061d0dbfd..cf5e5fafcef1f848f2d59703a2ac93c68ec61d92 100644 (file)
--- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/GlobalBundleScanningSchemaServiceImpl.java
+++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/GlobalBundleScanningSchemaServiceImpl.java
@@ -18,6 +18,8 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;

+import java.util.concurrent.atomic.AtomicReference;
+import javax.annotation.concurrent.GuardedBy;

 import org.opendaylight.controller.sal.core.api.model.SchemaService;
 import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.concepts.Registration;
 import org.opendaylight.controller.sal.core.api.model.SchemaService;
 import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.concepts.Registration;


@@ -41,6 +43,9 @@ import org.slf4j.LoggerFactory;
 public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvider, SchemaService, ServiceTrackerCustomizer<SchemaContextListener, SchemaContextListener>, AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(GlobalBundleScanningSchemaServiceImpl.class);
 
 public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvider, SchemaService, ServiceTrackerCustomizer<SchemaContextListener, SchemaContextListener>, AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(GlobalBundleScanningSchemaServiceImpl.class);
 

+    private static AtomicReference<GlobalBundleScanningSchemaServiceImpl> globalInstance = new AtomicReference<>();
+
+    @GuardedBy(value = "lock")

     private final ListenerRegistry<SchemaContextListener> listeners = new ListenerRegistry<>();
     private final URLSchemaContextResolver contextResolver = URLSchemaContextResolver.create("global-bundle");
     private final BundleScanner scanner = new BundleScanner();
     private final ListenerRegistry<SchemaContextListener> listeners = new ListenerRegistry<>();
     private final URLSchemaContextResolver contextResolver = URLSchemaContextResolver.create("global-bundle");
     private final BundleScanner scanner = new BundleScanner();


@@ -50,30 +55,30 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi
     private BundleTracker<Iterable<Registration>> bundleTracker;
     private boolean starting = true;
     private volatile boolean stopping;
     private BundleTracker<Iterable<Registration>> bundleTracker;
     private boolean starting = true;
     private volatile boolean stopping;

-    private static GlobalBundleScanningSchemaServiceImpl instance;
+    private final Object lock = new Object();

 
     private GlobalBundleScanningSchemaServiceImpl(final BundleContext context) {
         this.context = Preconditions.checkNotNull(context);
     }
 
 
     private GlobalBundleScanningSchemaServiceImpl(final BundleContext context) {
         this.context = Preconditions.checkNotNull(context);
     }
 

-    public synchronized static GlobalBundleScanningSchemaServiceImpl createInstance(final BundleContext ctx) {
-        Preconditions.checkState(instance == null);
-        instance = new GlobalBundleScanningSchemaServiceImpl(ctx);
+    public static GlobalBundleScanningSchemaServiceImpl createInstance(final BundleContext ctx) {
+        GlobalBundleScanningSchemaServiceImpl instance = new GlobalBundleScanningSchemaServiceImpl(ctx);
+        Preconditions.checkState(globalInstance.compareAndSet(null, instance));

         instance.start();
         return instance;
     }
 
         instance.start();
         return instance;
     }
 

-    public synchronized static GlobalBundleScanningSchemaServiceImpl getInstance() {
+    public static GlobalBundleScanningSchemaServiceImpl getInstance() {
+        GlobalBundleScanningSchemaServiceImpl instance = globalInstance.get();

         Preconditions.checkState(instance != null, "Global Instance was not instantiated");
         return instance;
     }
 
     @VisibleForTesting
         Preconditions.checkState(instance != null, "Global Instance was not instantiated");
         return instance;
     }
 
     @VisibleForTesting

-    public static synchronized void destroyInstance() {
-        try {
+    public static void destroyInstance() {
+        GlobalBundleScanningSchemaServiceImpl instance = globalInstance.getAndSet(null);
+        if(instance != null) {

             instance.close();
             instance.close();

-        } finally {
-            instance = null;

         }
     }
 
         }
     }
 


@@ -88,13 +93,20 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi
         listenerTracker = new ServiceTracker<>(context, SchemaContextListener.class, GlobalBundleScanningSchemaServiceImpl.this);
         bundleTracker = new BundleTracker<>(context, Bundle.RESOLVED | Bundle.STARTING |
                 Bundle.STOPPING | Bundle.ACTIVE, scanner);
         listenerTracker = new ServiceTracker<>(context, SchemaContextListener.class, GlobalBundleScanningSchemaServiceImpl.this);
         bundleTracker = new BundleTracker<>(context, Bundle.RESOLVED | Bundle.STARTING |
                 Bundle.STOPPING | Bundle.ACTIVE, scanner);

-        bundleTracker.open();

 
 

-        LOG.debug("BundleTracker.open() complete");
+        synchronized(lock) {
+            bundleTracker.open();
+
+            LOG.debug("BundleTracker.open() complete");
+
+            boolean hasExistingListeners = Iterables.size(listeners.getListeners()) > 0;
+            if(hasExistingListeners) {
+                tryToUpdateSchemaContext();
+            }
+        }

 
         listenerTracker.open();
         starting = false;
 
         listenerTracker.open();
         starting = false;

-        tryToUpdateSchemaContext();

 
         LOG.debug("start() complete");
     }
 
         LOG.debug("start() complete");
     }


@@ -125,12 +137,14 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi
     }
 
     @Override
     }
 
     @Override

-    public synchronized ListenerRegistration<SchemaContextListener> registerSchemaContextListener(final SchemaContextListener listener) {
-        Optional<SchemaContext> potentialCtx = contextResolver.getSchemaContext();
-        if(potentialCtx.isPresent()) {
-            listener.onGlobalContextUpdated(potentialCtx.get());
+    public ListenerRegistration<SchemaContextListener> registerSchemaContextListener(final SchemaContextListener listener) {
+        synchronized(lock) {
+            Optional<SchemaContext> potentialCtx = contextResolver.getSchemaContext();
+            if(potentialCtx.isPresent()) {
+                listener.onGlobalContextUpdated(potentialCtx.get());
+            }
+            return listeners.register(listener);

         }
         }

-        return listeners.register(listener);

     }
 
     @Override
     }
 
     @Override


@@ -148,7 +162,8 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi
         }
     }
 
         }
     }
 

-    private synchronized void updateContext(final SchemaContext snapshot) {
+    @GuardedBy(value = "lock")
+    private void notifyListeners(final SchemaContext snapshot) {

         Object[] services = listenerTracker.getServices();
         for (ListenerRegistration<SchemaContextListener> listener : listeners) {
             try {
         Object[] services = listenerTracker.getServices();
         for (ListenerRegistration<SchemaContextListener> listener : listeners) {
             try {


@@ -213,7 +228,7 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi
          */
 
         @Override
          */
 
         @Override

-        public synchronized void removedBundle(final Bundle bundle, final BundleEvent event, final Iterable<Registration> urls) {
+        public void removedBundle(final Bundle bundle, final BundleEvent event, final Iterable<Registration> urls) {

             for (Registration url : urls) {
                 try {
                     url.close();
             for (Registration url : urls) {
                 try {
                     url.close();


@@ -234,7 +249,7 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi
     }
 
     @Override
     }
 
     @Override

-    public synchronized SchemaContextListener addingService(final ServiceReference<SchemaContextListener> reference) {
+    public SchemaContextListener addingService(final ServiceReference<SchemaContextListener> reference) {

 
         SchemaContextListener listener = context.getService(reference);
         SchemaContext _ctxContext = getGlobalContext();
 
         SchemaContextListener listener = context.getService(reference);
         SchemaContext _ctxContext = getGlobalContext();


@@ -244,17 +259,20 @@ public class GlobalBundleScanningSchemaServiceImpl implements SchemaContextProvi
         return listener;
     }
 
         return listener;
     }
 

-    public synchronized void tryToUpdateSchemaContext() {
+    public void tryToUpdateSchemaContext() {

         if (starting || stopping) {
             return;
         }
         if (starting || stopping) {
             return;
         }

-        Optional<SchemaContext> schema = contextResolver.getSchemaContext();
-        if(schema.isPresent()) {
-            if(LOG.isDebugEnabled()) {
-                LOG.debug("Got new SchemaContext: # of modules {}", schema.get().getAllModuleIdentifiers().size());
-            }

 
 

-            updateContext(schema.get());
+        synchronized(lock) {
+            Optional<SchemaContext> schema = contextResolver.getSchemaContext();
+            if(schema.isPresent()) {
+                if(LOG.isDebugEnabled()) {
+                    LOG.debug("Got new SchemaContext: # of modules {}", schema.get().getAllModuleIdentifiers().size());
+                }
+
+                notifyListeners(schema.get());
+            }

         }
     }
 
         }
     }