You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@karaf.apache.org by gn...@apache.org on 2016/08/23 11:44:39 UTC

karaf git commit: KARAF-4652 - ConcurrentModificationException and NullPointerException when starting Karaf

Repository: karaf
Updated Branches:
  refs/heads/master 2de853be9 -> e8fd84c99


KARAF-4652 - ConcurrentModificationException and NullPointerException when starting Karaf

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/e8fd84c9
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/e8fd84c9
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/e8fd84c9

Branch: refs/heads/master
Commit: e8fd84c993fe208d2666cb71ff0495724927cdcf
Parents: 2de853b
Author: Guillaume Nodet <gn...@apache.org>
Authored: Tue Aug 23 12:36:30 2016 +0200
Committer: Guillaume Nodet <gn...@apache.org>
Committed: Tue Aug 23 13:44:35 2016 +0200

----------------------------------------------------------------------
 .../karaf/features/internal/osgi/Activator.java | 23 ++++++++++----------
 .../internal/service/FeaturesServiceImpl.java   | 14 +++++++-----
 .../karaf/features/FeaturesServiceTest.java     |  8 +++----
 .../service/FeaturesServiceImplTest.java        |  8 +++----
 .../karaf/util/tracker/BaseActivator.java       | 16 +++++++-------
 5 files changed, 37 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/e8fd84c9/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java b/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
index d18e881..f89dc70 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/osgi/Activator.java
@@ -123,16 +123,16 @@ public class Activator extends BaseActivator {
         }
 
         // RegionDigraph
-        digraph = DigraphHelper.loadDigraph(bundleContext);
-        register(ResolverHookFactory.class, digraph.getResolverHookFactory());
-        register(CollisionHook.class, CollisionHookHelper.getCollisionHook(digraph));
-        register(org.osgi.framework.hooks.bundle.FindHook.class, digraph.getBundleFindHook());
-        register(org.osgi.framework.hooks.bundle.EventHook.class, digraph.getBundleEventHook());
-        register(org.osgi.framework.hooks.service.FindHook.class, digraph.getServiceFindHook());
-        register(org.osgi.framework.hooks.service.EventHook.class, digraph.getServiceEventHook());
-        register(RegionDigraph.class, digraph);
-        digraphMBean = new StandardManageableRegionDigraph(digraph, "org.apache.karaf", bundleContext);
-        digraphMBean.registerMBean();
+        StandardRegionDigraph dg = digraph = DigraphHelper.loadDigraph(bundleContext);
+        register(ResolverHookFactory.class, dg.getResolverHookFactory());
+        register(CollisionHook.class, CollisionHookHelper.getCollisionHook(dg));
+        register(org.osgi.framework.hooks.bundle.FindHook.class, dg.getBundleFindHook());
+        register(org.osgi.framework.hooks.bundle.EventHook.class, dg.getBundleEventHook());
+        register(org.osgi.framework.hooks.service.FindHook.class, dg.getServiceFindHook());
+        register(org.osgi.framework.hooks.service.EventHook.class, dg.getServiceEventHook());
+        register(RegionDigraph.class, dg);
+        StandardManageableRegionDigraph dgmb = digraphMBean = new StandardManageableRegionDigraph(dg, "org.apache.karaf", bundleContext);
+        dgmb.registerMBean();
 
 
         FeatureFinder featureFinder = new FeatureFinder();
@@ -204,13 +204,14 @@ public class Activator extends BaseActivator {
         }
         featuresService = new FeaturesServiceImpl(
                 bundleContext.getBundle(),
+                bundleContext,
                 bundleContext.getBundle(0).getBundleContext(),
                 stateStorage,
                 featureFinder,
                 eventAdminListener,
                 configurationAdmin,
                 resolver,
-                digraph,
+                dg,
                 overrides,
                 featureResolutionRange,
                 bundleUpdateRange,

http://git-wip-us.apache.org/repos/asf/karaf/blob/e8fd84c9/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
index 5cc2813..7ade50c 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
@@ -184,6 +184,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
     private Map<String, Map<String, Feature>> featureCache;
 
     public FeaturesServiceImpl(Bundle bundle,
+                               BundleContext bundleContext,
                                BundleContext systemBundleContext,
                                StateStorage storage,
                                FeatureFinder featureFinder,
@@ -202,6 +203,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
                                int scheduleMaxRun,
                                String blacklisted) {
         this.bundle = bundle;
+        this.bundleContext = bundleContext;
         this.systemBundleContext = systemBundleContext;
         this.storage = storage;
         this.featureFinder = featureFinder;
@@ -227,6 +229,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
     }
 
     public FeaturesServiceImpl(Bundle bundle,
+                               BundleContext bundleContext,
                                BundleContext systemBundleContext,
                                StateStorage storage,
                                FeatureFinder featureFinder,
@@ -246,6 +249,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
                                String blacklisted,
                                boolean configCfgStore) {
         this.bundle = bundle;
+        this.bundleContext = bundleContext;
         this.systemBundleContext = systemBundleContext;
         this.storage = storage;
         this.featureFinder = featureFinder;
@@ -279,10 +283,10 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
 
     @SuppressWarnings({"unchecked", "rawtypes"})
     private void checkResolve() {
-        if (bundle == null) {
+        if (bundleContext == null) {
             return; // Most certainly in unit tests
         }
-        File resolveFile = bundle.getBundleContext().getDataFile("resolve");
+        File resolveFile = bundleContext.getDataFile("resolve");
         if (!resolveFile.exists()) {
             return;
         }
@@ -311,7 +315,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
     }
 
     private void writeResolve(Map<String, Set<String>> requestedFeatures, EnumSet<Option> options) throws IOException {
-        File resolveFile = bundle.getBundleContext().getDataFile("resolve");
+        File resolveFile = bundleContext.getDataFile("resolve");
         Map<String, Object> request = new HashMap<>();
         List<String> opts = new ArrayList<>();
         for (Option opt : options) {
@@ -350,8 +354,8 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
                     state.bundleChecksums.clear();
                 }
                 storage.save(state);
-                if (bundle != null) { // For tests, this should never happen at runtime
-                    DigraphHelper.saveDigraph(bundle.getBundleContext(), digraph);
+                if (bundleContext != null) { // For tests, this should never happen at runtime
+                    DigraphHelper.saveDigraph(bundleContext, digraph);
                 }
             }
         } catch (IOException e) {

http://git-wip-us.apache.org/repos/asf/karaf/blob/e8fd84c9/features/core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java b/features/core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java
index 2b3d846..40ce483 100644
--- a/features/core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/FeaturesServiceTest.java
@@ -349,7 +349,7 @@ public class FeaturesServiceTest extends TestBase {
                 + "  <feature name='f2' version='0.2'><bundle>bundle2</bundle></feature>"
                 + "</features>");
 
-        FeaturesServiceImpl svc = new FeaturesServiceImpl(null, null, new Storage(), null, null, null, resolver, null, null, null, null, null, null, null, 0, 0, 0, null);
+        FeaturesServiceImpl svc = new FeaturesServiceImpl(null, null, null, new Storage(), null, null, null, resolver, null, null, null, null, null, null, null, 0, 0, 0, null);
         svc.addRepository(uri);
 
         assertEquals(feature("f2", "0.2"), svc.getFeatures("f2", "[0.1,0.3)")[0]);
@@ -375,7 +375,7 @@ public class FeaturesServiceTest extends TestBase {
         expect(fsl.getStartLevel()).andReturn(100);
         replay(bundleContext, bundle, fsl);
 
-        FeaturesServiceImpl svc = new FeaturesServiceImpl(null, bundleContext, new Storage(), null, null, null, resolver, null, null, null, null, null, null, null, 0, 0, 0, null);
+        FeaturesServiceImpl svc = new FeaturesServiceImpl(null, null, bundleContext, new Storage(), null, null, null, resolver, null, null, null, null, null, null, null, 0, 0, 0, null);
         svc.addRepository(uri);
         try {
             List<String> features = new ArrayList<String>();
@@ -400,7 +400,7 @@ public class FeaturesServiceTest extends TestBase {
         URI uri = createTempRepo("<features name='test' xmlns='http://karaf.apache.org/xmlns/features/v1.0.0'>"
                 + "  <featur><bundle>somebundle</bundle></featur></features>");
 
-        FeaturesServiceImpl svc = new FeaturesServiceImpl(null, null, new Storage(), null, null, null, resolver, null, null, null, null, null, null, null, 0, 0, 0, null);
+        FeaturesServiceImpl svc = new FeaturesServiceImpl(null, null, null, new Storage(), null, null, null, resolver, null, null, null, null, null, null, null, 0, 0, 0, null);
         try {
             svc.addRepository(uri);
             fail("exception expected");
@@ -418,7 +418,7 @@ public class FeaturesServiceTest extends TestBase {
                 + "  <feature name='f1'><bundle>file:bundle1</bundle><bundle>file:bundle2</bundle></feature>"
                 + "</features>");
 
-        FeaturesServiceImpl svc = new FeaturesServiceImpl(null, null, new Storage(), null, null, null, resolver, null, null, null, null, null, null, null, 0, 0, 0, null);
+        FeaturesServiceImpl svc = new FeaturesServiceImpl(null, null, null, new Storage(), null, null, null, resolver, null, null, null, null, null, null, null, 0, 0, 0, null);
         svc.addRepository(uri);
         Feature[] features = svc.getFeatures("f1");
         Assert.assertEquals(1, features.length);

http://git-wip-us.apache.org/repos/asf/karaf/blob/e8fd84c9/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
index a549046..8e3cdfe 100644
--- a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
@@ -54,7 +54,7 @@ public class FeaturesServiceImplTest extends TestBase {
     public void testGetFeature() throws Exception {
         Feature transactionFeature = feature("transaction", "1.0.0");
         final Map<String, Map<String, Feature>> features = features(transactionFeature);
-        final FeaturesServiceImpl impl = new FeaturesServiceImpl(null, null, new Storage(), null, null, null, this.resolver, null, "", null, null, null, null, null, 0, 0, 0, null) {
+        final FeaturesServiceImpl impl = new FeaturesServiceImpl(null, null, null, new Storage(), null, null, null, this.resolver, null, "", null, null, null, null, null, 0, 0, 0, null) {
             protected Map<String,Map<String,Feature>> getFeatures() throws Exception {
                 return features;
             }
@@ -65,7 +65,7 @@ public class FeaturesServiceImplTest extends TestBase {
     
     @Test
     public void testGetFeatureStripVersion() throws Exception {
-        final FeaturesServiceImpl impl = new FeaturesServiceImpl(null, null, new Storage(), null, null, null, this.resolver, null, "", null, null, null, null, null, 0, 0, 0, null) {
+        final FeaturesServiceImpl impl = new FeaturesServiceImpl(null, null, null, new Storage(), null, null, null, this.resolver, null, "", null, null, null, null, null, 0, 0, 0, null) {
             protected Map<String,Map<String,Feature>> getFeatures() throws Exception {
                 return features(feature("transaction", "1.0.0"));
             }
@@ -79,7 +79,7 @@ public class FeaturesServiceImplTest extends TestBase {
     
     @Test
     public void testGetFeatureNotAvailable() throws Exception {
-        final FeaturesServiceImpl impl = new FeaturesServiceImpl(null, null, new Storage(), null, null, null, this.resolver, null, "", null, null, null, null, null, 0, 0, 0, null) {
+        final FeaturesServiceImpl impl = new FeaturesServiceImpl(null, null, null, new Storage(), null, null, null, this.resolver, null, "", null, null, null, null, null, 0, 0, 0, null) {
             protected Map<String,Map<String,Feature>> getFeatures() throws Exception {
                 return features(feature("transaction", "1.0.0"));
             }
@@ -93,7 +93,7 @@ public class FeaturesServiceImplTest extends TestBase {
                 feature("transaction", "1.0.0"),
                 feature("transaction", "2.0.0")
         );
-        final FeaturesServiceImpl impl = new FeaturesServiceImpl(null, null, new Storage(), null, null, null, this.resolver, null, "", null, null, null, null, null, 0, 0, 0, null) {
+        final FeaturesServiceImpl impl = new FeaturesServiceImpl(null, null, null, new Storage(), null, null, null, this.resolver, null, "", null, null, null, null, null, 0, 0, 0, null) {
             protected Map<String,Map<String,Feature>> getFeatures() throws Exception {
                 return features;
             }

http://git-wip-us.apache.org/repos/asf/karaf/blob/e8fd84c9/util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java
----------------------------------------------------------------------
diff --git a/util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java b/util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java
index 53c0c23..b6e457d 100644
--- a/util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java
+++ b/util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java
@@ -25,6 +25,8 @@ import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.ThreadPoolExecutor;
@@ -50,7 +52,7 @@ public class BaseActivator implements BundleActivator, Runnable {
 
     private long schedulerStopTimeout = TimeUnit.MILLISECONDS.convert(30, TimeUnit.SECONDS);
 
-    private List<ServiceRegistration> registrations;
+    private final Queue<ServiceRegistration> registrations = new ConcurrentLinkedQueue<>();
     private Map<String, SingleServiceTracker> trackers = new HashMap<>();
     private ServiceRegistration managedServiceRegistration;
     private Dictionary<String, ?> configuration;
@@ -120,11 +122,12 @@ public class BaseActivator implements BundleActivator, Runnable {
     }
 
     protected void doStop() {
-        if (registrations != null) {
-            for (ServiceRegistration reg : registrations) {
-                reg.unregister();
+        while (true) {
+            ServiceRegistration reg = registrations.poll();
+            if (reg == null) {
+                break;
             }
-            registrations = null;
+            reg.unregister();
         }
     }
 
@@ -357,9 +360,6 @@ public class BaseActivator implements BundleActivator, Runnable {
     }
 
     private void trackRegistration(ServiceRegistration registration) {
-        if (registrations == null) {
-            registrations = new ArrayList<>();
-        }
         registrations.add(registration);
     }