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); }