You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aries.apache.org by jw...@apache.org on 2013/02/07 20:34:36 UTC

svn commit: r1443681 - in /aries/trunk/subsystem: subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/ subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/

Author: jwross
Date: Thu Feb  7 19:34:36 2013
New Revision: 1443681

URL: http://svn.apache.org/r1443681
Log:
Bundle event hook NPE and unresolved persisted subsystems.

Fixed NPE in bundle event hook when processing asynchronous installed events whose bundle or origin bundle revision
was null because the bundle was uninstalled.

Fixed issue where persisted subsystems were registered in the RESOLVED state before they were actually resolved.

Added bundle event hook tests for null bundle and origin bundle revisions when processing asynchronous events.

Fixed issue in itests where subsystems other than root could be returned from utility method.

Modified:
    aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BundleEventHook.java
    aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/SubsystemResourceInstaller.java
    aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/BundleEventHookTest.java
    aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/SubsystemTest.java

Modified: aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BundleEventHook.java
URL: http://svn.apache.org/viewvc/aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BundleEventHook.java?rev=1443681&r1=1443680&r2=1443681&view=diff
==============================================================================
--- aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BundleEventHook.java (original)
+++ aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/BundleEventHook.java Thu Feb  7 19:34:36 2013
@@ -15,6 +15,7 @@ package org.apache.aries.subsystem.core.
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -43,7 +44,7 @@ public class BundleEventHook implements 
 	public void event(BundleEvent event, Collection<BundleContext> contexts) {
 		if ((event.getType() & (BundleEvent.INSTALLED | BundleEvent.UNINSTALLED)) == 0)
 			return;
-		// Protected against deadlock when the bundle event hook receives an
+		// Protect against deadlock when the bundle event hook receives an
 		// event before subsystems has fully initialized, in which case the
 		// events are queued and processed once initialization is complete.
 		synchronized (this) {
@@ -80,6 +81,12 @@ public class BundleEventHook implements 
 		return activator.getSubsystems();
 	}
 	
+	/*
+	 * Note that because some events may be processed asynchronously, we can no
+	 * longer rely on the guarantees that a synchronous event brings. For
+	 * example, bundle revisions adapted from bundles included in events may be
+	 * null.
+	 */
 	private void handleEvent(BundleEvent event) {
 		switch (event.getType()) {
 			case BundleEvent.INSTALLED:
@@ -93,13 +100,22 @@ public class BundleEventHook implements 
 		}
 	}
 	
+	/*
+	 * This method guards against an uninstalled origin bundle. Guards against a
+	 * null bundle revision are done elsewhere. It is assumed the bundle
+	 * revision is never null once we get here.
+	 */
 	private void handleExplicitlyInstalledBundleBundleContext(BundleRevision originRevision, BundleRevision bundleRevision) {
 		// The bundle needs to be associated with all subsystems that are 
 		// associated with the bundle whose context was used to install the 
 		// bundle.
 		Collection<BasicSubsystem> subsystems = getSubsystems().getSubsystemsReferencing(originRevision);
 		if (subsystems.isEmpty())
-			throw new IllegalStateException("Orphaned bundle revision detected: " + originRevision);
+			// If subsystems does not know about the origin bundle for some
+			// reason (e.g., the event is being processed asynchronously
+			// and the origin bundle has been uninstalled), associate the
+			// installed bundle with the root subsystem.
+			subsystems = Collections.singleton(getSubsystems().getRootSubsystem());
 		for (BasicSubsystem s : subsystems)
 			Utils.installResource(bundleRevision, s);
 	}
@@ -124,13 +140,17 @@ public class BundleEventHook implements 
 		BundleRevision originRevision = origin.adapt(BundleRevision.class);
 		Bundle bundle = event.getBundle();
 		BundleRevision bundleRevision = bundle.adapt(BundleRevision.class);
+		if (bundleRevision == null)
+			// The event is being processed asynchronously and the installed
+			// bundle has been uninstalled. Nothing we can do.
+			return;
 		bundleToRevision.put(bundle, bundleRevision);
 		// Only handle explicitly installed bundles. An explicitly installed
 		// bundle is a bundle that was installed using some other bundle's
 		// BundleContext or using RegionDigraph.
 		if (ThreadLocalSubsystem.get() != null)
 			return;
-		if ("org.eclipse.equionox.region".equals(origin.getSymbolicName()))
+		if ("org.eclipse.equinox.region".equals(origin.getSymbolicName()))
 			// The bundle was installed using RegionDigraph.
 			handleExplicitlyInstalledBundleRegionDigraph(origin, bundleRevision);
 		else

Modified: aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/SubsystemResourceInstaller.java
URL: http://svn.apache.org/viewvc/aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/SubsystemResourceInstaller.java?rev=1443681&r1=1443680&r2=1443681&view=diff
==============================================================================
--- aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/SubsystemResourceInstaller.java (original)
+++ aries/trunk/subsystem/subsystem-core/src/main/java/org/apache/aries/subsystem/core/internal/SubsystemResourceInstaller.java Thu Feb  7 19:34:36 2013
@@ -27,6 +27,8 @@ import org.osgi.service.coordinator.Part
 import org.osgi.service.repository.RepositoryContent;
 import org.osgi.service.subsystem.Subsystem.State;
 
+import com.sun.org.apache.bcel.internal.generic.GETSTATIC;
+
 public class SubsystemResourceInstaller extends ResourceInstaller {
 	public SubsystemResourceInstaller(Coordination coordination, Resource resource, BasicSubsystem subsystem) {
 		super(coordination, resource, subsystem);
@@ -83,17 +85,15 @@ public class SubsystemResourceInstaller 
 	}
 	
 	private BasicSubsystem installAriesSubsystem(BasicSubsystem subsystem) throws Exception {
-		// If the state is null, this is a brand new subsystem. If the state is
-		// not null, this is a persisted subsystem. For brand new subsystems,
-		// an INSTALLING event must be propagated.
-		if (subsystem.getState() == null)
-			subsystem.setState(State.INSTALLING);
 		addChild(subsystem);
 		addReference(subsystem);
 		addConstituent(subsystem);
 		addSubsystem(subsystem);
 		installRegionContextBundle(subsystem);
-		Activator.getInstance().getSubsystemServiceRegistrar().register(subsystem, this.subsystem);
+		// This will emit the initial service event for INSTALLING subsystems.
+		// The first event for RESOLVED (i.e. persisted) subsystems is emitted later.
+		if (State.INSTALLING.equals(subsystem.getState()))
+			Activator.getInstance().getSubsystemServiceRegistrar().register(subsystem, this.subsystem);
 		Comparator<Resource> comparator = new InstallResourceComparator();
 		// Install dependencies first...
 		List<Resource> dependencies = new ArrayList<Resource>(subsystem.getResource().getInstallableDependencies());
@@ -121,6 +121,9 @@ public class SubsystemResourceInstaller 
 		// in which case an INSTALLED event must be propagated.
 		if (State.INSTALLING.equals(subsystem.getState()))
 			subsystem.setState(State.INSTALLED);
+		else
+			// This is a persisted subsystem in the RESOLVED state. Emit the first service event.
+			Activator.getInstance().getSubsystemServiceRegistrar().register(subsystem, this.subsystem);
 		return subsystem;
 	}
 	

Modified: aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/BundleEventHookTest.java
URL: http://svn.apache.org/viewvc/aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/BundleEventHookTest.java?rev=1443681&r1=1443680&r2=1443681&view=diff
==============================================================================
--- aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/BundleEventHookTest.java (original)
+++ aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/BundleEventHookTest.java Thu Feb  7 19:34:36 2013
@@ -18,25 +18,36 @@
  */
 package org.apache.aries.subsystem.itests;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
 
+import org.eclipse.equinox.region.RegionDigraph;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.ops4j.pax.exam.junit.MavenConfiguredJUnit4TestRunner;
 import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleException;
 import org.osgi.framework.ServiceEvent;
 import org.osgi.framework.ServiceListener;
+import org.osgi.framework.ServiceReference;
+import org.osgi.framework.namespace.IdentityNamespace;
+import org.osgi.service.subsystem.Subsystem;
+import org.osgi.service.subsystem.SubsystemConstants;
 
 @RunWith(MavenConfiguredJUnit4TestRunner.class)
 public class BundleEventHookTest extends SubsystemTest {
@@ -44,6 +55,10 @@ public class BundleEventHookTest extends
 	 * Bundle-SymbolicName: bundle.a.jar
 	 */
 	private static final String BUNDLE_A = "bundle.a.jar";
+	/*
+	 * Bundle-SymbolicName: bundle.b.jar
+	 */
+	private static final String BUNDLE_B = "bundle.b.jar";
 	
 	@Before
 	public static void createApplications() throws Exception {
@@ -51,12 +66,17 @@ public class BundleEventHookTest extends
 			return;
 		}
 		createBundleA();
+		createBundleB();
 		createdApplications = true;
 	}
 	
 	private static void createBundleA() throws IOException {
 		createBundle(BUNDLE_A);
 	}
+	
+	private static void createBundleB() throws IOException {
+		createBundle(BUNDLE_B);
+	}
     
     /*
      * See https://issues.apache.org/jira/browse/ARIES-982.
@@ -124,4 +144,107 @@ public class BundleEventHookTest extends
     		executor.shutdownNow();
     	}
     }
+    
+    /*
+     * Because bundle events are queued for later asynchronous processing while
+     * the root subsystem is initializing, it is possible to see an installed
+     * event for a bundle that has been uninstalled (i.e. the bundle revision
+     * will be null). These events should be ignored.
+     */
+    @Test
+    public void testIgnoreUninstalledBundleInAsyncInstalledEvent() throws Exception {
+    	final Bundle core = getSubsystemCoreBundle();
+    	core.stop();
+    	final AtomicReference<Bundle> a = new AtomicReference<Bundle>();
+    	bundleContext.addServiceListener(
+    			new ServiceListener() {
+					@Override
+					public void serviceChanged(ServiceEvent event) {
+						if ((event.getType() & (ServiceEvent.REGISTERED | ServiceEvent.MODIFIED)) == 0)
+							return;
+						if (a.get() != null)
+							// We've been here before and already done what needs doing.
+							return;
+						ServiceReference<Subsystem> sr = (ServiceReference<Subsystem>)event.getServiceReference();
+						Subsystem s = bundleContext.getService(sr);
+						try {
+							// Queue up the installed event.
+							a.set(core.getBundleContext().installBundle(BUNDLE_A, new FileInputStream(BUNDLE_A)));
+							// Ensure the bundle will be uninstalled before the event is processed.
+							a.get().uninstall();
+						}
+						catch (Exception e) {
+							e.printStackTrace();
+						}
+					}
+    			}, 
+    			"(&(objectClass=org.osgi.service.subsystem.Subsystem)(subsystem.id=0)(subsystem.state=RESOLVED))");
+    	try {
+    		// Before the fix, this would fail due to an NPE resulting from a
+    		// null bundle revision.
+    		core.start();
+    	}
+    	catch (BundleException e) {
+    		e.printStackTrace();
+    		fail("Subsystems failed to handle an asynchronous bundle installed event after the bundle was uninstalled");
+    	}
+    	assertBundleState(a.get(), Bundle.UNINSTALLED);
+    	Subsystem root = getRootSubsystem();
+    	assertState(Subsystem.State.ACTIVE, root);
+    	assertNotConstituent(root, a.get().getSymbolicName());
+    }
+    
+    /*
+     * Because bundle events are queued for later asynchronous processing while
+     * the root subsystem is initializing, it is possible to see an installed
+     * event whose origin bundle has been uninstalled (i.e. the origin bundle's
+     * revision will be null). These events should result in the installed
+     * bundle being associated with the root subsystem.
+     */
+    @Test
+    public void testIgnoreUninstalledOriginBundleInAsyncInstalledEvent() throws Exception {
+    	final Bundle core = getSubsystemCoreBundle();
+    	core.stop();
+    	final Bundle b = bundleContext.installBundle(BUNDLE_B, new FileInputStream(BUNDLE_B));
+    	// Ensure bundle B has a context.
+    	b.start();
+    	final AtomicReference<Bundle> a = new AtomicReference<Bundle>();
+    	bundleContext.addServiceListener(
+    			new ServiceListener() {
+					@Override
+					public void serviceChanged(ServiceEvent event) {
+						if ((event.getType() & (ServiceEvent.REGISTERED | ServiceEvent.MODIFIED)) == 0)
+							return;
+						if (a.get() != null)
+							// We've been here before and already done what needs doing.
+							return;
+						ServiceReference<Subsystem> sr = (ServiceReference<Subsystem>)event.getServiceReference();
+						Subsystem s = bundleContext.getService(sr);
+						try {
+							// Queue up the installed event for bundle A using B's context.
+							a.set(b.getBundleContext().installBundle(BUNDLE_A, new FileInputStream(BUNDLE_A)));
+							// Ensure the origin bundle will be uninstalled before the event is processed.
+							b.uninstall();
+						}
+						catch (Exception e) {
+							e.printStackTrace();
+						}
+					}
+    			}, 
+    			"(&(objectClass=org.osgi.service.subsystem.Subsystem)(subsystem.id=0)(subsystem.state=RESOLVED))");
+    	try {
+    		// Before the fix, this would fail due to an NPE resulting from a
+    		// null bundle revision.
+    		core.start();
+    	}
+    	catch (BundleException e) {
+    		e.printStackTrace();
+    		fail("Subsystems failed to handle an asynchronous bundle installed event after the origin bundle was uninstalled");
+    	}
+    	assertBundleState(a.get(), Bundle.INSTALLED);
+    	assertBundleState(b, Bundle.UNINSTALLED);
+    	Subsystem root = getRootSubsystem();
+    	assertState(Subsystem.State.ACTIVE, root);
+    	assertConstituent(root, a.get().getSymbolicName());
+    }
 }

Modified: aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/SubsystemTest.java
URL: http://svn.apache.org/viewvc/aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/SubsystemTest.java?rev=1443681&r1=1443680&r2=1443681&view=diff
==============================================================================
--- aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/SubsystemTest.java (original)
+++ aries/trunk/subsystem/subsystem-itests/src/test/java/org/apache/aries/subsystem/itests/SubsystemTest.java Thu Feb  7 19:34:36 2013
@@ -715,7 +715,7 @@ public abstract class SubsystemTest exte
 	}
 	
 	protected Subsystem getRootSubsystem() {
-		return getOsgiService(Subsystem.class);
+		return getOsgiService(Subsystem.class, "(&(objectClass=org.osgi.service.subsystem.Subsystem)(subsystem.id=0))", DEFAULT_TIMEOUT);
 	}
 	
 	protected Subsystem getRootSubsystemInState(Subsystem.State state, long timeout) throws InterruptedException {