Bug 338697 - NullPointerException in BundleLoader.getImportedSourcesv20110404-1700

6 files changed, 172 insertions, 90 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java
index 90c3959f8..61b922132 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/services/resolver/StateResolverTest.java
@@ -17,6 +17,8 @@ import java.util.*;
 import junit.framework.Test;
 import junit.framework.TestSuite;
 import org.eclipse.osgi.framework.util.Headers;
+import org.eclipse.osgi.internal.resolver.StateImpl;
+import org.eclipse.osgi.internal.resolver.StateObjectFactoryImpl;
 import org.eclipse.osgi.service.resolver.*;
 import org.eclipse.osgi.tests.OSGiTestsActivator;
 import org.osgi.framework.*;
@@ -3271,6 +3273,84 @@ public class StateResolverTest extends AbstractStateTest {
 		assertTrue("2.0", a.getResolvedImports()[0].getExporter() == systemBundle2); //$NON-NLS-1$
 	}
 
+	public void testBug338697() throws BundleException, IOException {
+		State state = buildEmptyState();
+		int bundleID = 0;
+
+		Hashtable manifest = new Hashtable();
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "A"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0"); //$NON-NLS-1$
+		manifest.put(Constants.EXPORT_PACKAGE, "a");
+		BundleDescription a = state.getFactory().createBundleDescription(state, manifest, "A", bundleID++); //$NON-NLS-1$
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "B"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0"); //$NON-NLS-1$
+		manifest.put(Constants.IMPORT_PACKAGE, "a,d");
+		BundleDescription b = state.getFactory().createBundleDescription(state, manifest, "B", bundleID++); //$NON-NLS-1$
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "C"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0"); //$NON-NLS-1$
+		manifest.put(Constants.IMPORT_PACKAGE, "a,d");
+		BundleDescription c = state.getFactory().createBundleDescription(state, manifest, "C", bundleID++); //$NON-NLS-1$
+
+		state.addBundle(a);
+		state.addBundle(b);
+		state.addBundle(c);
+		state.resolve();
+
+		assertTrue("a", a.isResolved()); //$NON-NLS-1$
+		assertFalse("b", b.isResolved()); //$NON-NLS-1$
+		assertFalse("c", c.isResolved()); //$NON-NLS-1$
+
+		BundleContext context = OSGiTestsActivator.getContext();
+		File stateCache = context.getDataFile("statecache"); //$NON-NLS-1$
+		stateCache.mkdirs();
+		File stateFile = new File(stateCache, ".state");
+		File lazyFile = new File(stateCache, ".lazy");
+		StateObjectFactoryImpl factory = (StateObjectFactoryImpl) StateObjectFactory.defaultFactory;
+		factory.writeState(state, stateFile, lazyFile);
+		state = null;
+
+		StateImpl systemState = factory.readSystemState(null, stateFile, lazyFile, true, -1);
+		systemState.setResolver(platformAdmin.createResolver());
+		systemState.resolve(new BundleDescription[0]);
+
+		// call twice to force unload
+		systemState.unloadLazyData(systemState.getTimeStamp());
+		systemState.unloadLazyData(systemState.getTimeStamp());
+
+		manifest.clear();
+		manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_SYMBOLICNAME, "D"); //$NON-NLS-1$
+		manifest.put(Constants.BUNDLE_VERSION, "1.0"); //$NON-NLS-1$
+		manifest.put(Constants.EXPORT_PACKAGE, "d");
+		BundleDescription d = systemState.getFactory().createBundleDescription(systemState, manifest, "C", bundleID++); //$NON-NLS-1$
+		systemState.addBundle(d);
+		systemState.resolve();
+
+		BundleDescription[] bundles = systemState.getBundles();
+		for (int i = 0; i < bundles.length; i++) {
+			assertTrue("Bundle is not resolved: " + bundles[i], bundles[i].isResolved());
+			ExportPackageDescription[] imports = bundles[i].getResolvedImports();
+			for (int j = 0; j < imports.length; j++) {
+				assertNotNull("Bundle has a null import: " + bundles[i] + " " + j, imports[j]);
+				boolean found = false;
+				ExportPackageDescription[] exports = imports[j].getSupplier().getSelectedExports();
+				for (int k = 0; k < exports.length && !found; k++)
+					found = exports[k] == imports[j];
+				if (!found)
+					fail("Found duplicate export: " + imports[j]);
+			}
+		}
+	}
+
 	private ExportPackageDescription[] isConsistent(ExportPackageDescription[] pkgs1, ExportPackageDescription[] pkgs2) {
 		for (int i = 0; i < pkgs1.length; i++)
 			for (int j = 0; j < pkgs2.length; j++)
diff --git a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/internal/baseadaptor/StateManager.java b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/internal/baseadaptor/StateManager.java
index 910fd5a49..a927c69c9 100644
--- a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/internal/baseadaptor/StateManager.java
+++ b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/internal/baseadaptor/StateManager.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2010 IBM Corporation and others.
+ * Copyright (c) 2003, 2011 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -300,8 +300,8 @@ public class StateManager implements PlatformAdmin, Runnable {
 			}
 			if (systemState != null)
 				synchronized (systemState) {
-					if (timeStamp == systemState.getTimeStamp() && !systemState.dynamicCacheChanged())
-						systemState.unloadLazyData();
+					if (!systemState.unloadLazyData(timeStamp))
+						return;
 				}
 		}
 	}
diff --git a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/BundleDescriptionImpl.java b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/BundleDescriptionImpl.java
index efcc81c76..ab7f8993f 100644
--- a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/BundleDescriptionImpl.java
+++ b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/BundleDescriptionImpl.java
@@ -660,7 +660,7 @@ public final class BundleDescriptionImpl extends BaseDescriptionImpl implements
 		if (reader == null)
 			throw new IllegalStateException("No valid reader for the bundle description"); //$NON-NLS-1$
 
-		synchronized (reader) {
+		synchronized (currentState.monitor) {
 			if (isFullyLoaded()) {
 				reader.setAccessedFlag(true); // set reader accessed flag
 				return this.lazyData;
@@ -734,7 +734,7 @@ public final class BundleDescriptionImpl extends BaseDescriptionImpl implements
 		StateReader reader = currentState == null ? null : currentState.getReader();
 		if (reader == null)
 			throw new IllegalStateException("BundleDescription does not belong to a reader."); //$NON-NLS-1$
-		synchronized (reader) {
+		synchronized (currentState.monitor) {
 			if ((stateBits & LAZY_LOADED) == 0)
 				return;
 			if (!isFullyLoaded())
diff --git a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateImpl.java b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateImpl.java
index b84dec86f..231ec76a8 100644
--- a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateImpl.java
+++ b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateImpl.java
@@ -38,17 +38,17 @@ public abstract class StateImpl implements State {
 
 	transient private Resolver resolver;
 	transient private StateDeltaImpl changes;
-	transient volatile private boolean resolving = false;
+	transient private boolean resolving = false;
 	transient private LinkedList<BundleDescription> removalPendings = new LinkedList<BundleDescription>();
 
-	private volatile boolean resolved = true;
-	private volatile long timeStamp = System.currentTimeMillis();
+	private boolean resolved = true;
+	private long timeStamp = System.currentTimeMillis();
 	private final KeyedHashSet bundleDescriptions = new KeyedHashSet(false);
 	private final Map<BundleDescription, List<ResolverError>> resolverErrors = new HashMap<BundleDescription, List<ResolverError>>();
 	private StateObjectFactory factory;
 	private final KeyedHashSet resolvedBundles = new KeyedHashSet();
 	private final Map<BundleDescription, List<DisabledInfo>> disabledBundles = new HashMap<BundleDescription, List<DisabledInfo>>();
-	private volatile boolean fullyLoaded = false;
+	private boolean fullyLoaded = false;
 	private boolean dynamicCacheChanged = false;
 	// only used for lazy loading of BundleDescriptions
 	private StateReader reader;
@@ -62,7 +62,7 @@ public abstract class StateImpl implements State {
 
 	private static long cumulativeTime;
 
-	private final Object monitor = new Object();
+	final Object monitor = new Object();
 
 	// to prevent extra-package instantiation 
 	protected StateImpl() {
@@ -415,6 +415,7 @@ public abstract class StateImpl implements State {
 	}
 
 	private StateDelta resolve(boolean incremental, BundleDescription[] reResolve, BundleDescription[] triggers) {
+		fullyLoad();
 		synchronized (this.monitor) {
 			if (resolver == null)
 				throw new IllegalStateException("no resolver set"); //$NON-NLS-1$
@@ -423,7 +424,6 @@ public abstract class StateImpl implements State {
 			ResolverHook currentHook = null;
 			try {
 				resolving = true;
-				fullyLoad();
 				long start = 0;
 				if (StateManager.DEBUG_PLATFORM_ADMIN_RESOLVER)
 					start = System.currentTimeMillis();
@@ -1028,13 +1028,9 @@ public abstract class StateImpl implements State {
 
 	// not synchronized on this to prevent deadlock
 	public final void fullyLoad() {
-		// TODO add back if ee min 1.2 adds holdsLock method
-		//if (Thread.holdsLock(this.monitor)) {
-		//	throw new IllegalStateException("Should not call fullyLoad() holding monitor."); //$NON-NLS-1$
-		//}
-		if (reader == null)
-			return;
-		synchronized (reader) {
+		synchronized (this.monitor) {
+			if (reader == null)
+				return;
 			if (fullyLoaded == true)
 				return;
 			if (reader.isLazyLoaded())
@@ -1044,18 +1040,22 @@ public abstract class StateImpl implements State {
 	}
 
 	// not synchronized on this to prevent deadlock
-	public final void unloadLazyData() {
+	public final boolean unloadLazyData(long checkStamp) {
 		// make sure no other thread is trying to unload or load
-		synchronized (reader) {
+		synchronized (this.monitor) {
+			if (checkStamp != getTimeStamp() || dynamicCacheChanged())
+				return false;
 			if (reader.getAccessedFlag()) {
 				reader.setAccessedFlag(false); // reset accessed flag
-				return;
+				return true;
 			}
 			fullyLoaded = false;
 			BundleDescription[] bundles = getBundles();
 			for (int i = 0; i < bundles.length; i++)
 				((BundleDescriptionImpl) bundles[i]).unload();
 			reader.flushLazyObjectCache();
+			resolver.flush();
+			return true;
 		}
 	}
 
diff --git a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateReader.java b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateReader.java
index 575c1d8b7..f26ce5f2d 100644
--- a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateReader.java
+++ b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateReader.java
@@ -763,7 +763,7 @@ final class StateReader {
 		this.accessedFlag = accessedFlag;
 	}
 
-	synchronized void fullyLoad() {
+	void fullyLoad() {
 		setAccessedFlag(true);
 		DataInputStream in = null;
 		try {
@@ -782,7 +782,7 @@ final class StateReader {
 		}
 	}
 
-	synchronized void fullyLoad(BundleDescriptionImpl target) throws IOException {
+	void fullyLoad(BundleDescriptionImpl target) throws IOException {
 		setAccessedFlag(true);
 		DataInputStream in = null;
 		try {
diff --git a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateWriter.java b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateWriter.java
index ebff6e6ed..6ecc5960d 100644
--- a/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateWriter.java
+++ b/bundles/org.eclipse.osgi/resolver/src/org/eclipse/osgi/internal/resolver/StateWriter.java
@@ -94,76 +94,78 @@ class StateWriter {
 		DataOutputStream outState = null;
 		FileOutputStream fosLazy = null;
 		FileOutputStream fosState = null;
-		try {
-			BundleDescription[] bundles = state.getBundles();
-			StateHelperImpl.getInstance().sortBundles(bundles);
-			// need to prime the object table with all bundles
-			// this allows us to write only indexes to bundles in the lazy data
-			for (int i = 0; i < bundles.length; i++) {
-				addToObjectTable(bundles[i]);
-				if (bundles[i].getHost() != null)
-					addToObjectTable(bundles[i].getHost());
-			}
-			// first write the lazy data to get the offsets and sizes to the lazy data
-			fosLazy = new FileOutputStream(lazyFile);
-			outLazy = new DataOutputStream(new BufferedOutputStream(fosLazy));
-			for (int i = 0; i < bundles.length; i++)
-				writeBundleDescriptionLazyData(bundles[i], outLazy);
-			// now write the state data
-			fosState = new FileOutputStream(stateFile);
-			outState = new DataOutputStream(new BufferedOutputStream(fosState));
-			outState.write(StateReader.STATE_CACHE_VERSION);
-			if (writePrefix(state, outState))
-				return;
-			outState.writeLong(state.getTimeStamp());
-			// write the platform property keys
-			String[] platformPropKeys = state.getPlatformPropertyKeys();
-			writePlatformProp(platformPropKeys, outState);
-			// write the platform property values
-			Dictionary<Object, Object>[] propSet = state.getPlatformProperties();
-			outState.writeInt(propSet.length);
-			for (int i = 0; i < propSet.length; i++) {
-				Dictionary<Object, Object> props = propSet[i];
-				outState.writeInt(platformPropKeys.length);
-				for (int j = 0; j < platformPropKeys.length; j++)
-					writePlatformProp(props.get(platformPropKeys[j]), outState);
-			}
-			outState.writeInt(bundles.length);
-			for (int i = 0; i < bundles.length; i++)
-				// write out each bundle with the force flag set to make sure
-				// the data is written at least once in the non-lazy state data
-				writeBundleDescription(bundles[i], outState, true);
-			// write the DisabledInfos
-			DisabledInfo[] infos = state.getDisabledInfos();
-			outState.writeInt(infos.length);
-			for (int i = 0; i < infos.length; i++)
-				writeDisabledInfo(infos[i], outState);
-			outState.writeBoolean(state.isResolved());
-		} finally {
-			if (outLazy != null) {
-				try {
-					outLazy.flush();
-					fosLazy.getFD().sync();
-				} catch (IOException e) {
-					// do nothing, we tried
+		synchronized (state.monitor) {
+			try {
+				BundleDescription[] bundles = state.getBundles();
+				StateHelperImpl.getInstance().sortBundles(bundles);
+				// need to prime the object table with all bundles
+				// this allows us to write only indexes to bundles in the lazy data
+				for (int i = 0; i < bundles.length; i++) {
+					addToObjectTable(bundles[i]);
+					if (bundles[i].getHost() != null)
+						addToObjectTable(bundles[i].getHost());
 				}
-				try {
-					outLazy.close();
-				} catch (IOException e) {
-					// do nothing
+				// first write the lazy data to get the offsets and sizes to the lazy data
+				fosLazy = new FileOutputStream(lazyFile);
+				outLazy = new DataOutputStream(new BufferedOutputStream(fosLazy));
+				for (int i = 0; i < bundles.length; i++)
+					writeBundleDescriptionLazyData(bundles[i], outLazy);
+				// now write the state data
+				fosState = new FileOutputStream(stateFile);
+				outState = new DataOutputStream(new BufferedOutputStream(fosState));
+				outState.write(StateReader.STATE_CACHE_VERSION);
+				if (writePrefix(state, outState))
+					return;
+				outState.writeLong(state.getTimeStamp());
+				// write the platform property keys
+				String[] platformPropKeys = state.getPlatformPropertyKeys();
+				writePlatformProp(platformPropKeys, outState);
+				// write the platform property values
+				Dictionary<Object, Object>[] propSet = state.getPlatformProperties();
+				outState.writeInt(propSet.length);
+				for (int i = 0; i < propSet.length; i++) {
+					Dictionary<Object, Object> props = propSet[i];
+					outState.writeInt(platformPropKeys.length);
+					for (int j = 0; j < platformPropKeys.length; j++)
+						writePlatformProp(props.get(platformPropKeys[j]), outState);
 				}
-			}
-			if (outState != null) {
-				try {
-					outState.flush();
-					fosState.getFD().sync();
-				} catch (IOException e) {
-					// do nothing, we tried
+				outState.writeInt(bundles.length);
+				for (int i = 0; i < bundles.length; i++)
+					// write out each bundle with the force flag set to make sure
+					// the data is written at least once in the non-lazy state data
+					writeBundleDescription(bundles[i], outState, true);
+				// write the DisabledInfos
+				DisabledInfo[] infos = state.getDisabledInfos();
+				outState.writeInt(infos.length);
+				for (int i = 0; i < infos.length; i++)
+					writeDisabledInfo(infos[i], outState);
+				outState.writeBoolean(state.isResolved());
+			} finally {
+				if (outLazy != null) {
+					try {
+						outLazy.flush();
+						fosLazy.getFD().sync();
+					} catch (IOException e) {
+						// do nothing, we tried
+					}
+					try {
+						outLazy.close();
+					} catch (IOException e) {
+						// do nothing
+					}
 				}
-				try {
-					outState.close();
-				} catch (IOException e) {
-					// do nothing
+				if (outState != null) {
+					try {
+						outState.flush();
+						fosState.getFD().sync();
+					} catch (IOException e) {
+						// do nothing, we tried
+					}
+					try {
+						outState.close();
+					} catch (IOException e) {
+						// do nothing
+					}
 				}
 			}
 		}