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 <gno...@apache.org>
Authored: Tue Aug 23 12:36:30 2016 +0200
Committer: Guillaume Nodet <gno...@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);
     }
 

Reply via email to