Bug 349309 - Clean up code for initializing MRUBundleFileListv20110620

5 files changed, 106 insertions, 79 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java
index e618b5512..06fcfb686 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java
@@ -919,14 +919,22 @@ public class SystemBundleTests extends AbstractBundleTests {
 		assertEquals("Wrong stopEvent", FrameworkEvent.STOPPED, stopEvent.getType()); //$NON-NLS-1$
 	}
 
-	public void testBug253942() {
+	public void testMRUBundleFileList() {
+		doMRUBundleFileList(10);
+	}
+
+	//	public void testMRUBundleFileListExpectedToFail() {
+	//		doMRUBundleFileList(0);
+	//	}
+
+	private void doMRUBundleFileList(int limit) {
 		// create/start/stop/start/stop test
-		File config = OSGiTestsActivator.getContext().getDataFile("testBug253942"); //$NON-NLS-1$
+		File config = OSGiTestsActivator.getContext().getDataFile(getName()); //$NON-NLS-1$
 		Properties configuration = new Properties();
 		configuration.put(Constants.FRAMEWORK_STORAGE, config.getAbsolutePath());
-		configuration.put("osgi.bundlefile.limit", "10"); //$NON-NLS-1$//$NON-NLS-2$
+		configuration.put("osgi.bundlefile.limit", Integer.toString(limit)); //$NON-NLS-1$//$NON-NLS-2$
 
-		Equinox equinox = new Equinox(configuration);
+		final Equinox equinox = new Equinox(configuration);
 		try {
 			equinox.init();
 		} catch (BundleException e) {
@@ -945,7 +953,7 @@ public class SystemBundleTests extends AbstractBundleTests {
 
 		File[] testBundles = null;
 		try {
-			testBundles = createBundles(new File(config, "bundles"), 20); //$NON-NLS-1$
+			testBundles = createBundles(new File(config, "bundles"), 3000); //$NON-NLS-1$
 		} catch (IOException e) {
 			fail("Unexpected error creating budnles", e); //$NON-NLS-1$
 		}
@@ -973,18 +981,41 @@ public class SystemBundleTests extends AbstractBundleTests {
 		} catch (BundleException e) {
 			fail("Failed to start the framework", e); //$NON-NLS-1$
 		}
-		systemContext = equinox.getBundleContext();
-		Bundle[] bundles = systemContext.getBundles();
-		// get an entry from each bundle to ensure each one gets opened.
-		try {
-			for (int i = 0; i < bundles.length; i++) {
-				bundles[i].getEntry("/META-INF/MANIFEST.MF"); //$NON-NLS-1$
+
+		openAllBundleFiles(equinox.getBundleContext());
+
+		final Exception[] failureException = new BundleException[1];
+		final FrameworkEvent[] success = new FrameworkEvent[] {null};
+		Thread waitForUpdate = new Thread(new Runnable() {
+			public void run() {
+				try {
+					success[0] = equinox.waitForStop(10000);
+				} catch (InterruptedException e) {
+					failureException[0] = e;
+				}
 			}
-		} catch (Throwable t) {
-			// An exception used to get thrown here when we tried to close 
-			// the least used bundle file
-			fail("Failed to get bundle entries", t);
+		}, "test waitForStop thread"); //$NON-NLS-1$
+		waitForUpdate.start();
+		try {
+			// delay hack to allow waitForUpdate thread to block on waitForStop before we update.
+			Thread.sleep(100);
+		} catch (InterruptedException e) {
+			fail("unexpected interuption", e);
+		}
+		try {
+			equinox.update();
+		} catch (BundleException e) {
+			fail("Failed to update the framework", e); //$NON-NLS-1$
 		}
+		try {
+			waitForUpdate.join();
+		} catch (InterruptedException e) {
+			fail("unexpected interuption", e); //$NON-NLS-1$
+		}
+		if (failureException[0] != null)
+			fail("Error occurred while waiting", failureException[0]); //$NON-NLS-1$
+
+		openAllBundleFiles(equinox.getBundleContext());
 
 		try {
 			equinox.stop();
@@ -999,6 +1030,20 @@ public class SystemBundleTests extends AbstractBundleTests {
 		assertEquals("Wrong state for SystemBundle", Bundle.RESOLVED, equinox.getState()); //$NON-NLS-1$
 	}
 
+	private void openAllBundleFiles(BundleContext context) {
+		Bundle[] bundles = context.getBundles();
+		// get an entry from each bundle to ensure each one gets opened.
+		try {
+			for (int i = 0; i < bundles.length; i++) {
+				assertNotNull("No manifest for: " + bundles[i], bundles[i].getEntry("/META-INF/MANIFEST.MF"));
+			}
+		} catch (Throwable t) {
+			// An exception used to get thrown here when we tried to close 
+			// the least used bundle file
+			fail("Failed to get bundle entries", t);
+		}
+	}
+
 	public void testURLExternalFormat01() {
 		// create multiple instances test
 		File config1 = OSGiTestsActivator.getContext().getDataFile("testURLExternalFormat01_1"); //$NON-NLS-1$
diff --git a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/StartLevelManager.java b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/StartLevelManager.java
index 70ddca566..efd697c99 100644
--- a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/StartLevelManager.java
+++ b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/StartLevelManager.java
@@ -257,9 +257,9 @@ public class StartLevelManager implements EventDispatcher<Object, Object, StartL
 						decFWSL(i - 1, sortedBundles);
 					}
 					if (newSL == 0) {
-						// stop and unload all bundles
-						suspendAllBundles(framework.bundles);
+						// unload all bundles
 						unloadAllBundles(framework.bundles);
+						stopSystemBundle();
 					}
 				}
 				framework.publishFrameworkEvent(FrameworkEvent.STARTLEVEL_CHANGED, framework.systemBundle, null, listeners);
@@ -575,9 +575,6 @@ public class StartLevelManager implements EventDispatcher<Object, Object, StartL
 
 		saveActiveStartLevel(decToSL);
 
-		if (decToSL == 0) // stopping the framework
-			return;
-
 		// just decrementing the active startlevel - framework is not shutting down
 		// Do not check framework.isForcedRestart here because we want to stop the active bundles regardless.
 		for (int i = shutdown.length - 1; i >= 0; i--) {
@@ -598,29 +595,9 @@ public class StartLevelManager implements EventDispatcher<Object, Object, StartL
 	}
 
 	/**
-	 *  Suspends all bundles in the vector passed in.
-	 * @param bundles list of Bundle objects to be suspended
+	 * Stops the system bundle
 	 */
-	private void suspendAllBundles(BundleRepository bundles) {
-		boolean changed;
-		do {
-			changed = false;
-
-			AbstractBundle[] shutdown = this.getInstalledBundles(bundles, false);
-
-			// shutdown all running bundles
-			for (int i = shutdown.length - 1; i >= 0; i--) {
-				AbstractBundle bundle = shutdown[i];
-
-				if (framework.suspendBundle(bundle, false)) {
-					if (Debug.DEBUG_STARTLEVEL) {
-						Debug.println("SLL: stopped bundle " + bundle.getBundleId()); //$NON-NLS-1$
-					}
-					changed = true;
-				}
-			}
-		} while (changed);
-
+	private void stopSystemBundle() {
 		try {
 			framework.systemBundle.context.stop();
 		} catch (BundleException sbe) {
diff --git a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/MRUBundleFileList.java b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/MRUBundleFileList.java
index 40f75afef..8c59890a2 100644
--- a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/MRUBundleFileList.java
+++ b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/MRUBundleFileList.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2005, 2010 IBM Corporation and others.
+ * Copyright (c) 2005, 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
@@ -44,7 +44,7 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle
 	final private long[] useStampList;
 	// the limit of open files to allow before least used bundle file is closed
 	final private int fileLimit; // value < MIN will disable MRU
-	final private EventManager bundleFileCloserManager;
+	private EventManager bundleFileCloserManager = null;
 	final private Map<Object, Object> bundleFileCloser;
 	// the current number of open bundle files
 	private int numOpen = 0;
@@ -63,13 +63,11 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle
 		if (fileLimit >= MIN) {
 			this.bundleFileList = new BundleFile[fileLimit];
 			this.useStampList = new long[fileLimit];
-			this.bundleFileCloserManager = new EventManager("Bundle File Closer"); //$NON-NLS-1$
 			this.bundleFileCloser = new CopyOnWriteIdentityMap<Object, Object>();
 			this.bundleFileCloser.put(this, this);
 		} else {
 			this.bundleFileList = null;
 			this.useStampList = null;
-			this.bundleFileCloserManager = null;
 			this.bundleFileCloser = null;
 		}
 	}
@@ -84,6 +82,7 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle
 		if (fileLimit < MIN)
 			return; // MRU is disabled
 		BundleFile toRemove = null;
+		EventManager manager = null;
 		synchronized (this) {
 			if (bundleFile.getMruIndex() >= 0)
 				return; // do nothing; someone is trying add a bundleFile that is already in an MRU list
@@ -114,10 +113,16 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle
 			bundleFile.setMruIndex(index);
 			incUseStamp(index);
 			numOpen++;
+			if (toRemove != null) {
+				if (bundleFileCloserManager == null)
+					bundleFileCloserManager = new EventManager("Bundle File Closer"); //$NON-NLS-1$
+				manager = bundleFileCloserManager;
+			}
+
 		}
 		// must not close the toRemove bundle file while holding the lock of another bundle file (bug 161976)
 		// This queues the bundle file for close asynchronously.
-		closeBundleFile(toRemove);
+		closeBundleFile(toRemove, manager);
 	}
 
 	/**
@@ -188,12 +193,12 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle
 		}
 	}
 
-	private void closeBundleFile(BundleFile toRemove) {
+	private void closeBundleFile(BundleFile toRemove, EventManager manager) {
 		if (toRemove == null)
 			return;
 		try {
 			/* queue to hold set of listeners */
-			ListenerQueue<Object, Object, BundleFile> queue = new ListenerQueue<Object, Object, BundleFile>(bundleFileCloserManager);
+			ListenerQueue<Object, Object, BundleFile> queue = new ListenerQueue<Object, Object, BundleFile>(manager);
 			/* add bundle file closer to the queue */
 			queue.queueListeners(bundleFileCloser.entrySet(), this);
 			/* dispatch event to set of listeners */
@@ -209,8 +214,11 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle
 	 * Closes the bundle file closer thread for the MRU list
 	 */
 	public void shutdown() {
-		if (bundleFileCloserManager != null)
-			bundleFileCloserManager.close();
+		synchronized (this) {
+			if (bundleFileCloserManager != null)
+				bundleFileCloserManager.close();
+			bundleFileCloserManager = null;
+		}
 	}
 
 	/**
diff --git a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/internal/baseadaptor/BaseStorage.java b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/internal/baseadaptor/BaseStorage.java
index 93e06c987..b165bfe85 100644
--- a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/internal/baseadaptor/BaseStorage.java
+++ b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/internal/baseadaptor/BaseStorage.java
@@ -91,7 +91,7 @@ public class BaseStorage implements SynchronousBundleListener {
 	private static final String PERM_DATA_FILE = ".permdata"; //$NON-NLS-1$
 	private static final byte PERMDATA_VERSION = 1;
 
-	private MRUBundleFileList mruList = new MRUBundleFileList();
+	private final MRUBundleFileList mruList = new MRUBundleFileList();
 
 	BaseAdaptor adaptor;
 	// assume a file: installURL
@@ -442,9 +442,7 @@ public class BaseStorage implements SynchronousBundleListener {
 		return null;
 	}
 
-	void saveAllData(boolean shutdown) {
-		if (Debug.DEBUG_GENERAL)
-			Debug.println("Saving framework data ..."); //$NON-NLS-1$
+	private StorageManager getStorageManager() {
 		if (storageManagerClosed)
 			try {
 				storageManager.open(!LocationManager.getConfigurationLocation().isReadOnly());
@@ -454,6 +452,12 @@ public class BaseStorage implements SynchronousBundleListener {
 				FrameworkLogEntry logEntry = new FrameworkLogEntry(FrameworkAdaptor.FRAMEWORK_SYMBOLICNAME, FrameworkLogEntry.ERROR, 0, message, 0, e, null);
 				adaptor.getFrameworkLog().log(logEntry);
 			}
+		return storageManager;
+	}
+
+	void saveAllData(boolean shutdown) {
+		if (Debug.DEBUG_GENERAL)
+			Debug.println("Saving framework data ..."); //$NON-NLS-1$
 		saveBundleDatas();
 		saveStateData(shutdown);
 		savePermissionStorage();
@@ -512,7 +516,7 @@ public class BaseStorage implements SynchronousBundleListener {
 		if (Debug.DEBUG_GENERAL)
 			Debug.println("About to save permission data ..."); //$NON-NLS-1$
 		try {
-			ManagedOutputStream fmos = storageManager.getOutputStream(PERM_DATA_FILE);
+			ManagedOutputStream fmos = getStorageManager().getOutputStream(PERM_DATA_FILE);
 			DataOutputStream out = new DataOutputStream(new BufferedOutputStream(fmos));
 			boolean error = true;
 			try {
@@ -565,7 +569,7 @@ public class BaseStorage implements SynchronousBundleListener {
 		if (Debug.DEBUG_GENERAL)
 			Debug.println("Saving bundle data ..."); //$NON-NLS-1$
 		try {
-			ManagedOutputStream fmos = storageManager.getOutputStream(LocationManager.BUNDLE_DATA_FILE);
+			ManagedOutputStream fmos = getStorageManager().getOutputStream(LocationManager.BUNDLE_DATA_FILE);
 			DataOutputStream out = new DataOutputStream(new BufferedOutputStream(fmos));
 			boolean error = true;
 			try {
@@ -660,9 +664,10 @@ public class BaseStorage implements SynchronousBundleListener {
 				synchronized (stateManager) {
 					stateManager.update(stateTmpFile, lazyTmpFile);
 				}
-			storageManager.lookup(LocationManager.STATE_FILE, true);
-			storageManager.lookup(LocationManager.LAZY_FILE, true);
-			storageManager.update(new String[] {LocationManager.STATE_FILE, LocationManager.LAZY_FILE}, new String[] {stateTmpFile.getName(), lazyTmpFile.getName()});
+			StorageManager curStorageManager = getStorageManager();
+			curStorageManager.lookup(LocationManager.STATE_FILE, true);
+			curStorageManager.lookup(LocationManager.LAZY_FILE, true);
+			curStorageManager.update(new String[] {LocationManager.STATE_FILE, LocationManager.LAZY_FILE}, new String[] {stateTmpFile.getName(), lazyTmpFile.getName()});
 		} catch (IOException e) {
 			adaptor.getFrameworkLog().log(new FrameworkEvent(FrameworkEvent.ERROR, context.getBundle(), e));
 		} finally {
@@ -738,7 +743,7 @@ public class BaseStorage implements SynchronousBundleListener {
 			if (isDirectory(data, base, file))
 				result = new DirBundleFile(file);
 			else
-				result = new ZipBundleFile(file, data, getMRUList());
+				result = new ZipBundleFile(file, data, mruList);
 		}
 
 		if (result == null && content instanceof String) {
@@ -762,12 +767,6 @@ public class BaseStorage implements SynchronousBundleListener {
 		return result;
 	}
 
-	private synchronized MRUBundleFileList getMRUList() {
-		if (mruList == null)
-			mruList = new MRUBundleFileList();
-		return mruList;
-	}
-
 	private boolean isDirectory(BaseData data, boolean base, File file) {
 		if (!base)
 			// there is no other place to check this; just consult the file directly
@@ -857,10 +856,11 @@ public class BaseStorage implements SynchronousBundleListener {
 	}
 
 	private File[] findStorageFiles(String[] fileNames) {
+		StorageManager curStorageManager = getStorageManager();
 		File[] storageFiles = new File[fileNames.length];
 		try {
 			for (int i = 0; i < storageFiles.length; i++)
-				storageFiles[i] = storageManager.lookup(fileNames[i], false);
+				storageFiles[i] = curStorageManager.lookup(fileNames[i], false);
 		} catch (IOException ex) {
 			if (Debug.DEBUG_GENERAL) {
 				Debug.println("Error reading state file " + ex.getMessage()); //$NON-NLS-1$
@@ -896,7 +896,7 @@ public class BaseStorage implements SynchronousBundleListener {
 				//it did not exist in either place, so create it in the original location
 				if (!isReadOnly()) {
 					for (int i = 0; i < storageFiles.length; i++)
-						storageFiles[i] = storageManager.lookup(fileNames[i], true);
+						storageFiles[i] = curStorageManager.lookup(fileNames[i], true);
 				}
 			} catch (IOException ex) {
 				if (Debug.DEBUG_GENERAL) {
@@ -924,10 +924,7 @@ public class BaseStorage implements SynchronousBundleListener {
 		storageManagerClosed = true;
 		if (extensionListener != null)
 			context.removeBundleListener(extensionListener);
-		MRUBundleFileList current = mruList;
-		if (current != null)
-			current.shutdown();
-		mruList = null;
+		mruList.shutdown();
 		stateManager = null;
 	}
 
@@ -946,7 +943,7 @@ public class BaseStorage implements SynchronousBundleListener {
 	private InputStream findStorageStream(String fileName) {
 		InputStream storageStream = null;
 		try {
-			storageStream = storageManager.getInputStream(fileName);
+			storageStream = getStorageManager().getInputStream(fileName);
 		} catch (IOException ex) {
 			if (Debug.DEBUG_GENERAL) {
 				Debug.println("Error reading framework metadata: " + ex.getMessage()); //$NON-NLS-1$
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 a927c69c9..167385a95 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
@@ -59,13 +59,13 @@ public class StateManager implements PlatformAdmin, Runnable {
 	private long expireTime = 300000; // default to five minutes
 	private long readStartupTime;
 	private StateImpl systemState;
-	private StateObjectFactoryImpl factory;
+	private final StateObjectFactoryImpl factory;
 	private long lastTimeStamp;
 	private boolean cachedState = false;
-	private File stateFile;
-	private File lazyFile;
-	private long expectedTimeStamp;
-	private BundleContext context;
+	private final File stateFile;
+	private final File lazyFile;
+	private final long expectedTimeStamp;
+	private final BundleContext context;
 	private Thread dataManagerThread;
 
 	/**