Fix ModuleFactoryBundleTracker shutdown hang

If karaf is shutdown quickly after starting, the
ModuleFactoryBundleTracker#getModuleFactoryEntries method may
hang for a while trying to obtain BundleContexts. This has
been seen on jenkins with some feature tests. The ModuleFactoryBundleTracker
does a 1 min busy wait trying to obtain the BundleContext. This was done b/c the
tracker listens for RESOLVED bundles and the BundleContext isn't available
until after the bundle is started. So the busy wait was intended for startup
when bundles transition from RESOLVED -> STARTING. Once obtained, the
BundleContext is cached.

This works fine normally when all bundles start up. However, if stopped
quickly, some bundles may not have started, ie they remain in the
RESOLVED state with null BundleContext, so on shutdown when someone
calls getModuleFactoryEntries, it will busy wait and eventually timeout which
can take minutes.

What we need to do is remove the ModuleFactory entries when bundles are
stopped. The ModuleFactoryBundleTracker#removedBundle method does this
but it wasn't called on shutdown b/c it tracks RESOLVED, STARTING, ACTIVE
and STOPPING states. The solution is to remove STOPPING from the tracked states
so removedBundle will get called on transition ACTIVE -> STOPPING.
However, when transition STOPPING -> RESOLVED occurs the bundle will get
added back to the tracker and we don't want to re-add the ModuleFactory
entries. To prevent this it checks for BundleEvent type STOPPED.

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

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java
index 7fbd488eb39a4b4e52bda7d67777ade4901da7a8..ab52e8f1431e57ee2e6f864aa21f496dc8dae7e5 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ConfigManagerActivator.java
@@ -66,7 +66,7 @@ public class ConfigManagerActivator implements BundleActivator {
 
             // start extensible tracker
             ExtensibleBundleTracker<?> bundleTracker = new ExtensibleBundleTracker<>(context,
-                Bundle.RESOLVED | Bundle.STARTING | Bundle.STOPPING | Bundle.ACTIVE,
+                Bundle.RESOLVED | Bundle.STARTING | Bundle.ACTIVE,
                 primaryModuleFactoryBundleTracker, moduleInfoBundleTracker);
             bundleTracker.open();
 

diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTracker.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTracker.java
index 51443d01e8b7b9b2cbecd878b68fa35dd46827a9..1ece5627e456193ccfc7fd637eabfe976013a4fe 100644 (file)
--- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTracker.java
+++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTracker.java
@@ -56,6 +56,15 @@ public class ModuleFactoryBundleTracker implements BundleTrackerCustomizer<Objec
 
     @Override
     public Object addingBundle(Bundle bundle, BundleEvent event) {
+        if(event != null && (event.getType() == BundleEvent.STOPPED || event.getType() == BundleEvent.STOPPING)) {
+            // We're tracking RESOLVED, STARTING, and ACTIVE states but not STOPPING. So when a bundle transitions
+            // to STOPPING, removedBundle gets called and we remove the ModuleFactory entries. However the
+            // bundle will then transition to RESOLVED which will cause the tracker to notify addingBundle.
+            // In this case we don't want to re-add the ModuleFactory entries so we check if the BundleEvent
+            // is STOPPED.
+            return null;
+        }
+
         URL resource = bundle.getEntry("META-INF/services/" + ModuleFactory.class.getName());
         LOG.trace("Got addingBundle event of bundle {}, resource {}, event {}",
                 bundle, resource, event);
@@ -83,7 +92,9 @@ public class ModuleFactoryBundleTracker implements BundleTrackerCustomizer<Objec
 
     @Override
     public void removedBundle(Bundle bundle, BundleEvent event, Object object) {
-        bundleModuleFactoryMap.removeAll(new BundleKey(bundle));
+        synchronized (bundleModuleFactoryMap) {
+            bundleModuleFactoryMap.removeAll(new BundleKey(bundle));
+        }
 
         // workaround for service tracker not getting removed service event
         blankTransactionServiceTracker.blankTransaction();

diff --git a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTrackerTest.java b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTrackerTest.java
index 9ac570e2eed166045ac7930425b77932b5229263..da585b7151365eec3529161530403371f0016283 100644 (file)
--- a/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTrackerTest.java
+++ b/opendaylight/config/config-manager/src/test/java/org/opendaylight/controller/config/manager/impl/osgi/ModuleFactoryBundleTrackerTest.java
@@ -18,6 +18,7 @@ import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
 import java.util.Collection;
@@ -134,7 +135,7 @@ public class ModuleFactoryBundleTrackerTest {
     public void testBundleAddAndRemove() throws Exception {
         final ModuleFactoryBundleTracker tracker = new ModuleFactoryBundleTracker(blankTxTracker);
         doReturn(getClass().getResource("/module-factories/module-factory-ok")).when(bundle).getEntry(anyString());
-        tracker.addingBundle(bundle, mock(BundleEvent.class));
+        tracker.addingBundle(bundle, null);
 
         Collection<Entry<ModuleFactory, BundleContext>> entries = tracker.getModuleFactoryEntries();
         assertNotNull(entries);
@@ -145,13 +146,23 @@ public class ModuleFactoryBundleTrackerTest {
 
         doNothing().when(blankTxTracker).blankTransaction();;
 
-        tracker.removedBundle(bundle, mock(BundleEvent.class), bundle);
+        BundleEvent mockEvent = mock(BundleEvent.class);
+        doReturn(BundleEvent.STOPPING).when(mockEvent).getType();
+
+        tracker.removedBundle(bundle, mockEvent, bundle);
 
         entries = tracker.getModuleFactoryEntries();
         assertNotNull(entries);
         assertEquals(0, entries.size());
 
-        verify(blankTxTracker).blankTransaction();;
+        verify(blankTxTracker).blankTransaction();
+
+        reset(mockEvent);
+        doReturn(BundleEvent.STOPPED).when(mockEvent).getType();
+
+        tracker.addingBundle(bundle, mockEvent);
+
+        assertEquals(0, tracker.getModuleFactoryEntries().size());
     }
 
     @Test