More refactoring for FELIX-851:
* Modified locking protocol so that acquiring the global lock does not wait
for bundle locks, although a thread with the global lock must still wait
to lock bundles which are already locked.
* Modified acquiring a bundle lock so that it always throws an exception if
the bundle is not in a desired state, which makes it so we don't have to
do double-checking after acquiring the lock.
* Made bundle version available from IModule, which cleaned up some version
handling in various places.
* Modified boot delegation parsing to occur once in Felix with results passed
into modules so we only have one copy.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@745200 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
index 8e63108..eaeb73f 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
@@ -33,7 +33,7 @@
class BundleImpl implements Bundle
{
// No one should use this field directly, use getFramework() instead.
- private final Felix _m_felix;
+ private final Felix __m_felix;
private final BundleArchive m_archive;
private IModule[] m_modules = new IModule[0];
@@ -61,7 +61,7 @@
BundleImpl(Felix felix, BundleArchive archive) throws Exception
{
- _m_felix = felix;
+ __m_felix = felix;
m_archive = archive;
m_state = Bundle.INSTALLED;
m_stale = false;
@@ -83,7 +83,7 @@
// not access the field directly.
Felix getFramework()
{
- return _m_felix;
+ return __m_felix;
}
synchronized void dispose()
@@ -950,7 +950,9 @@
Long.toString(getBundleId()) + "." + Integer.toString(revision),
headerMap,
m_archive.getRevision(revision).getContent(),
- getFramework().getBundleStreamHandler());
+ getFramework().getBundleStreamHandler(),
+ getFramework().getBootPackages(),
+ getFramework().getBootPackageWildcards());
// Verify that the bundle symbolic name + version is unique.
if (module.getManifestVersion().equals("2"))
diff --git a/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java b/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
index 4af230f..ec51660 100644
--- a/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
+++ b/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
@@ -49,13 +49,13 @@
import org.apache.felix.moduleloader.IContent;
import org.apache.felix.moduleloader.IModule;
import org.apache.felix.moduleloader.IURLPolicy;
-import org.apache.felix.moduleloader.ResourceNotFoundException;
import org.osgi.framework.AdminPermission;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleActivator;
import org.osgi.framework.BundleContext;
import org.osgi.framework.BundleException;
import org.osgi.framework.Constants;
+import org.osgi.framework.Version;
/**
* The ExtensionManager class is used in several ways.
@@ -613,9 +613,13 @@
class ExtensionManagerModule extends ModuleImpl
{
+ private final Version m_version;
ExtensionManagerModule(Felix felix) throws BundleException
{
- super(m_logger, null, null, felix, "0", null, null, null);
+ super(m_logger, null, null, felix, "0", null, null, null,
+ felix.getBootPackages(), felix.getBootPackageWildcards());
+ m_version = new Version((String)
+ felix.getConfig().get(FelixConstants.FELIX_VERSION_PROPERTY));
}
public Map getHeaders()
@@ -633,6 +637,11 @@
return FelixConstants.SYSTEM_BUNDLE_SYMBOLICNAME;
}
+ public Version getVersion()
+ {
+ return m_version;
+ }
+
public Class getClassByDelegation(String name) throws ClassNotFoundException
{
if (!m_exportNames.contains(Util.getClassPackage(name)))
diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java
index a49397a..a2d7d2b 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -58,12 +58,11 @@
// Lock object used to determine if an individual bundle
// lock or the global lock can be acquired.
private final Object[] m_bundleLock = new Object[0];
- // Maps a thread object to a single-element int array to
- // keep track of how many locks the thread has acquired.
- private final Map m_lockingThreadMap = new HashMap();
- // Separately keeps track of how many times the global
- // lock was acquired by a given thread; if this value is
- // zero, then it means the global lock is free.
+ // The thread currently holding the global lock.
+ private Thread m_globalLockThread = null;
+ // How many times the global lock was acquired by the thread holding
+ // the global lock; if this value is zero, then it means the global
+ // lock is free.
private int m_globalLockCount = 0;
// Maps a bundle location to a bundle location;
@@ -115,6 +114,10 @@
private String m_executionEnvironment = "";
private Set m_executionEnvironmentCache = new HashSet();
+ // Boot package delegation.
+ private final String[] m_bootPkgs;
+ private final boolean[] m_bootPkgWildcards;
+
// Shutdown thread.
private Thread m_shutdownThread = null;
private ThreadGate m_shutdownGate = null;
@@ -295,6 +298,26 @@
// definition for creating the system bundle module.
m_extensionManager = new ExtensionManager(m_logger, this);
addModule(m_extensionManager.getModule());
+
+
+ // Read the boot delegation property and parse it.
+ String s = (m_configMap == null)
+ ? null
+ : (String) m_configMap.get(Constants.FRAMEWORK_BOOTDELEGATION);
+ s = (s == null) ? "java.*" : s + ",java.*";
+ StringTokenizer st = new StringTokenizer(s, " ,");
+ m_bootPkgs = new String[st.countTokens()];
+ m_bootPkgWildcards = new boolean[m_bootPkgs.length];
+ for (int i = 0; i < m_bootPkgs.length; i++)
+ {
+ s = st.nextToken();
+ if (s.endsWith("*"))
+ {
+ m_bootPkgWildcards[i] = true;
+ s = s.substring(0, s.length() - 1);
+ }
+ m_bootPkgs[i] = s;
+ }
}
Logger getLogger()
@@ -322,6 +345,16 @@
return m_bundleStreamHandler;
}
+ String[] getBootPackages()
+ {
+ return m_bootPkgs;
+ }
+
+ boolean[] getBootPackageWildcards()
+ {
+ return m_bootPkgWildcards;
+ }
+
private Map createUnmodifiableMap(Map mutableMap)
{
Map result = Collections.unmodifiableMap(mutableMap);
@@ -1075,11 +1108,6 @@
try
{
- if (bundle.getState() == Bundle.UNINSTALLED)
- {
- throw new IllegalArgumentException("Bundle is uninstalled.");
- }
-
if (startLevel >= 1)
{
impl.setStartLevel(startLevel);
@@ -1314,29 +1342,11 @@
void startBundle(BundleImpl bundle, boolean record) throws BundleException
{
// CONCURRENCY NOTE:
-// TODO: REFACTOR - This concurrency note will no longer be accurate.
- // Starting a bundle may actually impact many bundles, since
- // the bundle being started my need to be resolved, which in
- // turn may need to resolve other bundles. Despite this fact,
- // we only acquire the lock for the bundle being started, because
- // when resolve is called on this bundle, it will eventually
- // call resolve on the module loader search policy, which does
- // its own locking on the module factory instance. Since the
- // resolve algorithm is locking the module factory instance, it
- // is not possible for other bundles to be installed or removed,
- // so we don't have to worry about these possibilities.
- //
- // Further, if other bundles are started during this operation,
- // then either they will resolve first because they got the lock
- // on the module factory or we will resolve first since we got
- // the lock on the module factory, so there should be no interference.
- // If other bundles are stopped or uninstalled, this should pose
- // no problems, since this does not impact their resolved state.
- // If a refresh occurs, then the refresh algorithm ulimately has
- // to acquire the module factory instance lock too before it can
- // completely purge old modules, so it should also complete either
- // before or after this bundle is started. At least that's the
- // theory.
+ // We will first acquire the bundle lock for the specific bundle
+ // as long as the bundle is INSTALLED, RESOLVED, or ACTIVE. If this
+ // bundle is not yet resolved, then it will be resolved too. In
+ // that case, the global lock will be acquired to make sure no
+ // bundles can be installed or uninstalled during the resolve.
// Acquire bundle lock.
try
@@ -1356,7 +1366,6 @@
+ " cannot be started, since it is either starting or stopping.");
}
}
-
try
{
// The spec doesn't say whether it is possible to start an extension
@@ -1519,12 +1528,7 @@
// Variable to indicate whether bundle is active or not.
Throwable rethrow = null;
- // Cannot update an uninstalled bundle.
final int oldState = bundle.getState();
- if (oldState == Bundle.UNINSTALLED)
- {
- throw new IllegalStateException("The bundle is uninstalled.");
- }
// First get the update-URL from our header.
String updateLocation = (String)
@@ -1837,11 +1841,6 @@
try
{
- if (bundle.getState() == Bundle.UNINSTALLED)
- {
- throw new IllegalStateException("Bundle " + bundle + " is uninstalled.");
- }
-
// Extension Bundles are not removed until the framework is shutdown
if (bundle.isExtension())
{
@@ -1905,18 +1904,8 @@
// Fire UNINSTALLED event without holding the lock.
fireBundleEvent(BundleEvent.UNINSTALLED, bundle);
- // Acquire bundle lock again to check if we can auto-refresh.
- try
- {
- acquireBundleLock(bundle, Bundle.UNINSTALLED);
- }
- catch (IllegalStateException ex)
- {
- // Auto-refreshing is not required, so if we get an exception
- // here, we should probably just return, but this should never
- // happen.
- return;
- }
+ // Acquire global lock to check if we should auto-refresh.
+ acquireGlobalLock();
try
{
@@ -1938,8 +1927,8 @@
}
finally
{
- // Always release bundle lock.
- releaseBundleLock(bundle);
+ // Always release the global lock.
+ releaseGlobalLock();
}
}
@@ -3240,7 +3229,7 @@
// Acquire bundle lock.
try
{
- acquireBundleLock(bundle, Bundle.INSTALLED);
+ acquireBundleLock(bundle, Bundle.INSTALLED | Bundle.RESOLVED);
}
catch (IllegalStateException ex)
{
@@ -4109,6 +4098,15 @@
}
}
+ /**
+ * This method acquires the lock for the specified bundle as long as the
+ * bundle is in one of the specified states. If it is not, an exception
+ * is thrown. Bundle state changes will be monitored to avoid deadlocks.
+ * @param bundle The bundle to lock.
+ * @param desiredStates Logically OR'ed desired bundle states.
+ * @throws java.lang.IllegalStateException If the bundle is not in one of the
+ * specified desired states.
+ **/
void acquireBundleLock(BundleImpl bundle, int desiredStates)
throws IllegalStateException
{
@@ -4118,8 +4116,7 @@
// or if any thread has the global lock, unless the current thread
// holds the global lock.
while (!bundle.isLockable() ||
- ((m_globalLockCount > 0)
- && !m_lockingThreadMap.containsKey(Thread.currentThread())))
+ ((m_globalLockThread != null) && (m_globalLockThread != Thread.currentThread())))
{
// Check to make sure the bundle is in a desired state.
// If so, keep waiting. If not, throw an exception.
@@ -4137,59 +4134,46 @@
}
}
- // Increment the current thread's lock count.
- int[] counter = (int[]) m_lockingThreadMap.get(Thread.currentThread());
- if (counter == null)
+ // Now that we can acquire the bundle lock, let's check to make sure
+ // it is in a desired state; if not, throw an exception and do not
+ // lock it.
+ if ((desiredStates & bundle.getState()) == 0)
{
- counter = new int[] { 0 };
+ throw new IllegalStateException("Bundle in unexpected state.");
}
- counter[0]++;
- m_lockingThreadMap.put(Thread.currentThread(), counter);
// Acquire the bundle lock.
bundle.lock();
}
}
+ /**
+ * Releases the bundle's lock.
+ * @param bundle The bundle whose lock is to be released.
+ * @throws java.lang.IllegalStateException If the calling thread does not
+ * own the bundle lock.
+ **/
void releaseBundleLock(BundleImpl bundle)
{
synchronized (m_bundleLock)
{
- // Decrement the current thread's lock count.
- int[] counter = (int[]) m_lockingThreadMap.get(Thread.currentThread());
- if (counter != null)
- {
- counter[0]--;
- if (counter[0] == 0)
- {
- m_lockingThreadMap.remove(Thread.currentThread());
- }
- else
- {
- m_lockingThreadMap.put(Thread.currentThread(), counter);
- }
- }
- else
- {
- m_logger.log(Logger.LOG_ERROR, "Thread released a lock it doesn't own: " + bundle);
- }
-
// Unlock the bundle.
bundle.unlock();
m_bundleLock.notifyAll();
}
}
+ /**
+ * Acquires the global lock, which is necessary for multi-bundle operations
+ * like refreshing and resolving.
+ **/
private void acquireGlobalLock()
{
synchronized (m_bundleLock)
{
- // Wait as long as there are multiple threads holding locks,
- // but proceed if there are no lock holders or the current thread
- // is the only lock holder.
- while ((m_lockingThreadMap.size() > 1)
- || ((m_lockingThreadMap.size() == 1)
- && !m_lockingThreadMap.containsKey(Thread.currentThread())))
+ // Wait as long as some other thread holds the global lock.
+ while ((m_globalLockThread != null)
+ && (m_globalLockThread != Thread.currentThread()))
{
try
{
@@ -4201,48 +4185,36 @@
}
}
- // Increment the current thread's lock count.
- int[] counter = (int[]) m_lockingThreadMap.get(Thread.currentThread());
- if (counter == null)
- {
- counter = new int[] { 0 };
- }
- counter[0]++;
- m_lockingThreadMap.put(Thread.currentThread(), counter);
-
// Increment the current thread's global lock count.
m_globalLockCount++;
+ m_globalLockThread = Thread.currentThread();
}
}
+ /**
+ * Releases the global lock.
+ * @throws java.lang.IllegalStateException If the calling thread does not
+ * own the global lock.
+ **/
private void releaseGlobalLock()
{
- // Always unlock any locked bundles.
synchronized (m_bundleLock)
{
- // Decrement the current thread's lock count.
- int[] counter = (int[]) m_lockingThreadMap.get(Thread.currentThread());
- if (counter != null)
+ // Decrement the current thread's global lock count;
+ if (m_globalLockThread == Thread.currentThread())
{
- counter[0]--;
- if (counter[0] == 0)
+ m_globalLockCount--;
+ if (m_globalLockCount == 0)
{
- m_lockingThreadMap.remove(Thread.currentThread());
+ m_globalLockThread = null;
+ m_bundleLock.notifyAll();
}
- else
- {
- m_lockingThreadMap.put(Thread.currentThread(), counter);
- }
- counter = new int[] { 0 };
}
else
{
- m_logger.log(Logger.LOG_ERROR, "Thread release a lock it doesn't own.");
+ throw new IllegalStateException(
+ "The current thread doesn't own the global lock.");
}
-
- // Decrement the current thread's global lock count;
- m_globalLockCount--;
- m_bundleLock.notifyAll();
}
}
}
\ No newline at end of file
diff --git a/framework/src/main/java/org/apache/felix/framework/PackageAdminImpl.java b/framework/src/main/java/org/apache/felix/framework/PackageAdminImpl.java
index 16cdcf4..cd7bc38 100644
--- a/framework/src/main/java/org/apache/felix/framework/PackageAdminImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/PackageAdminImpl.java
@@ -101,9 +101,7 @@
**/
public Bundle[] getBundles(String symbolicName, String versionRange)
{
-// TODO: PACKAGEADMIN - This could be made more efficient by reducing object creation.
- VersionRange vr = (versionRange == null)
- ? null : VersionRange.parse(versionRange);
+ VersionRange vr = (versionRange == null) ? null : VersionRange.parse(versionRange);
Bundle[] bundles = m_felix.getBundles();
List list = new ArrayList();
for (int i = 0; (bundles != null) && (i < bundles.length); i++)
@@ -111,9 +109,7 @@
String sym = bundles[i].getSymbolicName();
if ((sym != null) && sym.equals(symbolicName))
{
- String s = (String) ((BundleImpl) bundles[i])
- .getCurrentModule().getHeaders().get(Constants.BUNDLE_VERSION);
- Version v = (s == null) ? new Version("0.0.0") : new Version(s);
+ Version v = ((BundleImpl) bundles[i]).getCurrentModule().getVersion();
if ((vr == null) || vr.isInRange(v))
{
list.add(bundles[i]);
@@ -128,12 +124,8 @@
Arrays.sort(bundles,new Comparator() {
public int compare(Object o1, Object o2)
{
- String s1 = (String) ((BundleImpl) o1)
- .getCurrentModule().getHeaders().get(Constants.BUNDLE_VERSION);
- String s2 = (String) ((BundleImpl) o2)
- .getCurrentModule().getHeaders().get(Constants.BUNDLE_VERSION);
- Version v1 = (s1 == null) ? new Version("0.0.0") : new Version(s1);
- Version v2 = (s2 == null) ? new Version("0.0.0") : new Version(s2);
+ Version v1 = ((BundleImpl) o1).getCurrentModule().getVersion();
+ Version v2 = ((BundleImpl) o2).getCurrentModule().getVersion();
// Compare in reverse order to get descending sort.
return v2.compareTo(v1);
}
diff --git a/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java b/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java
index 6c1e4ba..c24c512 100644
--- a/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java
@@ -102,7 +102,8 @@
public ModuleImpl(
Logger logger, Map configMap, FelixResolver resolver,
Bundle bundle, String id, Map headerMap, IContent content,
- URLStreamHandler streamHandler)
+ URLStreamHandler streamHandler, String[] bootPkgs,
+ boolean[] bootPkgWildcards)
throws BundleException
{
m_logger = logger;
@@ -113,6 +114,8 @@
m_headerMap = headerMap;
m_content = content;
m_streamHandler = streamHandler;
+ m_bootPkgs = bootPkgs;
+ m_bootPkgWildcards = bootPkgWildcards;
// We need to special case the system bundle module, which does not
// have a content.
@@ -185,28 +188,7 @@
m_nativeLibraries = null;
m_symbolicName = null;
}
-
- // Read the boot delegation property and parse it.
-// TODO: REFACTOR - This used to be per framework, now it is per module
-// which doesn't really make sense. Maybe pass in the arrays.
- String s = (m_configMap == null)
- ? null
- : (String) m_configMap.get(Constants.FRAMEWORK_BOOTDELEGATION);
- s = (s == null) ? "java.*" : s + ",java.*";
- StringTokenizer st = new StringTokenizer(s, " ,");
- m_bootPkgs = new String[st.countTokens()];
- m_bootPkgWildcards = new boolean[m_bootPkgs.length];
- for (int i = 0; i < m_bootPkgs.length; i++)
- {
- s = st.nextToken();
- if (s.endsWith("*"))
- {
- m_bootPkgWildcards[i] = true;
- s = s.substring(0, s.length() - 1);
- }
- m_bootPkgs[i] = s;
- }
- }
+ }
//
// Metadata access methods.
@@ -492,8 +474,6 @@
throws ClassNotFoundException, ResourceNotFoundException
{
// First, try to resolve the originating module.
-// TODO: FRAMEWORK - Consider opimizing this call to resolve, since it is called
-// for each class load.
try
{
m_resolver.resolve(this);
@@ -656,8 +636,6 @@
List completeUrlList = new ArrayList();
// First, try to resolve the originating module.
-// TODO: FRAMEWORK - Consider opimizing this call to resolve, since it is called
-// for each class load.
try
{
m_resolver.resolve(this);
diff --git a/framework/src/main/java/org/apache/felix/framework/searchpolicy/Resolver.java b/framework/src/main/java/org/apache/felix/framework/searchpolicy/Resolver.java
index 18774ab..c6258f1 100644
--- a/framework/src/main/java/org/apache/felix/framework/searchpolicy/Resolver.java
+++ b/framework/src/main/java/org/apache/felix/framework/searchpolicy/Resolver.java
@@ -1707,7 +1707,6 @@
public static interface ResolverState
{
IModule[] getModules();
- // TODO: RESOLVER - This should be on module.
Map getPotentialFragments(IModule module);
List getPotentialHosts(IModule module);
PackageSource[] getResolvedCandidates(IRequirement req);
diff --git a/framework/src/main/java/org/apache/felix/moduleloader/IModule.java b/framework/src/main/java/org/apache/felix/moduleloader/IModule.java
index 55cfe5b..ed1e897 100644
--- a/framework/src/main/java/org/apache/felix/moduleloader/IModule.java
+++ b/framework/src/main/java/org/apache/felix/moduleloader/IModule.java
@@ -25,6 +25,7 @@
import java.util.Map;
import org.apache.felix.framework.util.manifestparser.R4Library;
import org.osgi.framework.Bundle;
+import org.osgi.framework.Version;
public interface IModule
{
@@ -34,6 +35,7 @@
// Metadata access methods.
Map getHeaders();
String getSymbolicName();
+ Version getVersion();
ICapability[] getCapabilities();
IRequirement[] getRequirements();
IRequirement[] getDynamicRequirements();