From: Robert Varga Date: Thu, 27 Apr 2017 16:38:37 +0000 (+0200) Subject: Lazily create schema context in GlobalBundleScanning* X-Git-Tag: release/boron-sr4~5 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=mdsal.git;a=commitdiff_plain;h=98bc46847cda68e350c3d498e2bd718f7f82cafa 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 Signed-off-by: Robert Varga (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 caec7cc785..818667bf93 100644 --- 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, AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(OsgiBundleScanningSchemaService.class); + private static AtomicReference globalInstance = new AtomicReference<>(); + + @GuardedBy(value = "lock") private final ListenerRegistry 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> 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 - registerSchemaContextListener(final SchemaContextListener listener) { - final Optional potentialCtx = contextResolver.getSchemaContext(); - if (potentialCtx.isPresent()) { - listener.onGlobalContextUpdated(potentialCtx.get()); + public ListenerRegistration registerSchemaContextListener( + final SchemaContextListener listener) { + synchronized (lock) { + final Optional 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 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 urls) { + public void removedBundle(final Bundle bundle, final BundleEvent event, final Iterable 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 reference) { + public SchemaContextListener addingService(final ServiceReference 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 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 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 a90cf14042..46f689cdc0 100644 --- 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());