Lazily create schema context in GlobalBundleScanning*

On OsgiBundleScanningSchemaService 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>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 62ca5284757904ee8a2ac0f79f07b0f4c41dc830)

diff --git a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/osgi/OsgiBundleScanningSchemaService.java b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/osgi/OsgiBundleScanningSchemaService.java
index caec7cc7851b9982a450eded533782085deb4d80..818667bf9339da8a88355969b0275805e54bf7aa 100644 (file)
--- a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/osgi/OsgiBundleScanningSchemaService.java
+++ b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/osgi/OsgiBundleScanningSchemaService.java
@@ -19,6 +19,8 @@ import java.util.ArrayList;
 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.mdsal.dom.api.DOMSchemaService;
 import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.concepts.Registration;
@@ -42,6 +44,9 @@ public class OsgiBundleScanningSchemaService implements SchemaContextProvider, D
         ServiceTrackerCustomizer<SchemaContextListener, SchemaContextListener>, AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(OsgiBundleScanningSchemaService.class);
 
+    private static AtomicReference<OsgiBundleScanningSchemaService> globalInstance = new AtomicReference<>();
+
+    @GuardedBy(value = "lock")
     private final ListenerRegistry<SchemaContextListener> listeners = new ListenerRegistry<>();
     private final YangTextSchemaContextResolver contextResolver = YangTextSchemaContextResolver.create("global-bundle");
     private final BundleScanner scanner = new BundleScanner();
@@ -51,30 +56,30 @@ public class OsgiBundleScanningSchemaService implements SchemaContextProvider, D
     private BundleTracker<Iterable<Registration>> bundleTracker;
     private boolean starting = true;
     private volatile boolean stopping;
-    private static OsgiBundleScanningSchemaService instance;
+    private final Object lock = new Object();
 
     private OsgiBundleScanningSchemaService(final BundleContext context) {
         this.context = Preconditions.checkNotNull(context);
     }
 
-    public static synchronized OsgiBundleScanningSchemaService createInstance(final BundleContext ctx) {
-        Preconditions.checkState(instance == null);
-        instance = new OsgiBundleScanningSchemaService(ctx);
+    public static OsgiBundleScanningSchemaService createInstance(final BundleContext ctx) {
+        OsgiBundleScanningSchemaService instance = new OsgiBundleScanningSchemaService(ctx);
+        Preconditions.checkState(globalInstance.compareAndSet(null, instance));
         instance.start();
         return instance;
     }
 
-    public static synchronized OsgiBundleScanningSchemaService getInstance() {
+    public static OsgiBundleScanningSchemaService getInstance() {
+        OsgiBundleScanningSchemaService instance = globalInstance.get();
         Preconditions.checkState(instance != null, "Global Instance was not instantiated");
         return instance;
     }
 
     @VisibleForTesting
-    public static synchronized void destroyInstance() {
-        try {
+    public static void destroyInstance() {
+        OsgiBundleScanningSchemaService instance = globalInstance.getAndSet(null);
+        if (instance != null) {
             instance.close();
-        } finally {
-            instance = null;
         }
     }
 
@@ -89,15 +94,21 @@ public class OsgiBundleScanningSchemaService implements SchemaContextProvider, D
         listenerTracker = new ServiceTracker<>(context, SchemaContextListener.class,
                 OsgiBundleScanningSchemaService.this);
         bundleTracker = new BundleTracker<>(context, Bundle.RESOLVED | Bundle.STARTING
-                |
-                Bundle.STOPPING | Bundle.ACTIVE, scanner);
-        bundleTracker.open();
+                | Bundle.STOPPING | Bundle.ACTIVE, scanner);
+
+        synchronized (lock) {
+            bundleTracker.open();
 
-        LOG.debug("BundleTracker.open() complete");
+            LOG.debug("BundleTracker.open() complete");
+
+            boolean hasExistingListeners = Iterables.size(listeners.getListeners()) > 0;
+            if (hasExistingListeners) {
+                tryToUpdateSchemaContext();
+            }
+        }
 
         listenerTracker.open();
         starting = false;
-        tryToUpdateSchemaContext();
 
         LOG.debug("start() complete");
     }
@@ -118,13 +129,15 @@ public class OsgiBundleScanningSchemaService implements SchemaContextProvider, D
     }
 
     @Override
-    public synchronized ListenerRegistration<SchemaContextListener>
-        registerSchemaContextListener(final SchemaContextListener listener) {
-        final Optional<SchemaContext> potentialCtx = contextResolver.getSchemaContext();
-        if (potentialCtx.isPresent()) {
-            listener.onGlobalContextUpdated(potentialCtx.get());
+    public ListenerRegistration<SchemaContextListener> registerSchemaContextListener(
+            final SchemaContextListener listener) {
+        synchronized (lock) {
+            final Optional<SchemaContext> potentialCtx = contextResolver.getSchemaContext();
+            if (potentialCtx.isPresent()) {
+                listener.onGlobalContextUpdated(potentialCtx.get());
+            }
+            return listeners.register(listener);
         }
-        return listeners.register(listener);
     }
 
     @Override
@@ -143,7 +156,9 @@ public class OsgiBundleScanningSchemaService implements SchemaContextProvider, D
     }
 
     @SuppressWarnings("checkstyle:IllegalCatch")
-    private synchronized void updateContext(final SchemaContext snapshot) {
+    @VisibleForTesting
+    @GuardedBy(value = "lock")
+    void notifyListeners(final SchemaContext snapshot) {
         final Object[] services = listenerTracker.getServices();
         for (final ListenerRegistration<SchemaContextListener> listener : listeners) {
             try {
@@ -209,8 +224,7 @@ public class OsgiBundleScanningSchemaService implements SchemaContextProvider, D
          */
         @SuppressWarnings("checkstyle:IllegalCatch")
         @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 (final Registration url : urls) {
                 try {
                     url.close();
@@ -232,7 +246,7 @@ public class OsgiBundleScanningSchemaService implements SchemaContextProvider, D
     }
 
     @Override
-    public synchronized SchemaContextListener addingService(final ServiceReference<SchemaContextListener> reference) {
+    public SchemaContextListener addingService(final ServiceReference<SchemaContextListener> reference) {
 
         final SchemaContextListener listener = context.getService(reference);
         final SchemaContext _ctxContext = getGlobalContext();
@@ -242,17 +256,20 @@ public class OsgiBundleScanningSchemaService implements SchemaContextProvider, D
         return listener;
     }
 
-    public synchronized void tryToUpdateSchemaContext() {
+    public void tryToUpdateSchemaContext() {
         if (starting || stopping) {
             return;
         }
-        final 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) {
+            final 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());
+            }
         }
     }
 

diff --git a/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/osgi/OsgiBundleScanningSchemaServiceTest.java b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/osgi/OsgiBundleScanningSchemaServiceTest.java
index a90cf1404292c23eb12a8dd3ea1ef344b060ae70..46f689cdc00264079ec9d4e760896930dba22abd 100644 (file)
--- a/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/osgi/OsgiBundleScanningSchemaServiceTest.java
+++ b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/osgi/OsgiBundleScanningSchemaServiceTest.java
@@ -16,7 +16,6 @@ import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 
-import java.lang.reflect.Method;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -66,10 +65,7 @@ public class OsgiBundleScanningSchemaServiceTest {
         doNothing().when(schemaContextListener).onGlobalContextUpdated(schemaContext);
         osgiService.registerSchemaContextListener(schemaContextListener);
 
-        final Method schemaContextUpdate =
-                OsgiBundleScanningSchemaService.class.getDeclaredMethod("updateContext", SchemaContext.class);
-        schemaContextUpdate.setAccessible(true);
-        schemaContextUpdate.invoke(osgiService, schemaContext);
+        osgiService.notifyListeners(schemaContext);
 
         osgiService.registerSchemaContextListener(schemaContextListener);
         assertNull(osgiService.getSchemaContext());