Fix extension bundles (FELIX-969).

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@755045 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 a017bde..49291a9 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
@@ -49,12 +49,6 @@
     // Indicates whether the bundle is stale, meaning that it has
     // been refreshed and completely removed from the framework.
     private boolean m_stale = false;
-
-    // Indicates whether the bundle is an extension, meaning that it is
-    // installed as an extension bundle to the framework (i.e., can not be
-    // removed or updated until a framework restart.
-    private boolean m_extension = false;
-
     // Used for bundle locking.
     private int m_lockCount = 0;
     private Thread m_lockThread = null;
@@ -108,22 +102,30 @@
 
     synchronized void refresh() throws Exception
     {
-        // Dispose of the current modules.
-        dispose();
+        if (isExtension() && (getFramework().getState() != Bundle.STOPPING))
+        {
+            getFramework().getLogger().log(Logger.LOG_WARNING,
+                "Framework restart on extension bundle refresh not implemented.");
+        }
+        else
+        {
+            // Dispose of the current modules.
+            dispose();
 
-        // Now we will purge all old revisions, only keeping the newest one.
-        m_archive.purge();
+            // Now we will purge all old revisions, only keeping the newest one.
+            m_archive.purge();
 
-        // Lastly, we want to reset our bundle be reinitializing our state
-        // and recreating a module for the newest revision.
-        m_modules = new IModule[0];
-        final IModule module = createModule();
-        addModule(module);
-        m_state = Bundle.INSTALLED;
-        m_stale = false;
-        m_cachedHeaders.clear();
-        m_cachedHeadersTimestamp = 0;
-        m_removalPending = false;
+            // Lastly, we want to reset our bundle be reinitializing our state
+            // and recreating a module for the newest revision.
+            m_modules = new IModule[0];
+            final IModule module = createModule();
+            addModule(module);
+            m_state = Bundle.INSTALLED;
+            m_stale = false;
+            m_cachedHeaders.clear();
+            m_cachedHeadersTimestamp = 0;
+            m_removalPending = false;
+        }
     }
 
     synchronized BundleActivator getActivator()
@@ -703,12 +705,14 @@
 
     synchronized boolean isExtension()
     {
-        return m_extension;
-    }
-
-    synchronized void setExtension(boolean extension)
-    {
-        m_extension = extension;
+        for (int i = (m_modules.length - 1); i > -1; i--)
+        {
+            if (m_modules[i].isExtension())
+            {
+                return true;
+            }
+        }
+        return false;
     }
 
     public String getSymbolicName()
@@ -928,7 +932,8 @@
         SecurityProvider sp = getFramework().getSecurityProvider();
         if (sp != null)
         {
-            sp.checkBundle(this);
+            // TODO: Security
+            // sp.checkBundle(this);
         }
         module.setSecurityContext(new BundleProtectionDomain(getFramework(), this));
 
@@ -937,9 +942,15 @@
         dest[m_modules.length] = module;
         m_modules = dest;
 
-        // Now that the module is added to the bundle, we can update
-        // the resolver's module state.
-        getFramework().getResolverState().addModule(module);
+        // TODO: REFACTOR - consider moving ModuleImpl into the framework package
+        // so we can null module capabilities for extension bundles so we don't
+        // need this check anymore.
+        if (!isExtension())
+        {
+            // Now that the module is added to the bundle, we can update
+            // the resolver's module state.
+            getFramework().getResolverState().addModule(module);
+        }
     }
 
     private IModule createModule() throws Exception
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 2408e04..fde5379 100644
--- a/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
+++ b/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
@@ -285,11 +285,12 @@
         }
 
         // Not sure whether this is a good place to do it but we need to lock
-        felix.acquireBundleLock(felix, Bundle.ACTIVE);
+        felix.acquireBundleLock(felix, Bundle.RESOLVED | Bundle.STARTING | Bundle.ACTIVE);
 
         try
         {
-            bundle.setExtension(true);
+// TODO: EXTENSIONMANAGER - Should we be setting this?
+//            bundle.setExtension(true);
 
             // Merge the exported packages with the exported packages of the systembundle.
             ICapability[] exports = null;
@@ -328,7 +329,8 @@
         }
         catch (Exception ex)
         {
-            bundle.setExtension(false);
+// TODO: EXTENSIONMANAGER - Should we be setting this?
+//            bundle.setExtension(false);
             throw ex;
         }
         finally
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 5126c62..90403f9 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -1577,6 +1577,7 @@
                     throw new BundleException(
                         "Cannot acquire global lock to update the bundle.");
                 }
+                boolean wasExtension = bundle.isExtension();
                 try
                 {
 // REFACTOR - This adds the module to the resolver state, but should we do the
@@ -1601,20 +1602,16 @@
                     }
 
                     // If this is an update from a normal to an extension bundle
-                    // then attach the extension or else if this already is
-                    // an extension bundle then don't allow it to be resolved
-                    // again as per spec.
-                    if (!bundle.isExtension() &&
-                        Util.isExtensionBundle(
-                            bundle.getCurrentModule().getHeaders()))
+                    // then attach the extension
+                    if (!wasExtension && bundle.isExtension())
                     {
                         m_extensionManager.addExtensionBundle(this, bundle);
 // TODO: REFACTOR - Perhaps we could move this into extension manager.
-                        m_resolverState.refreshSystemBundleModule(bundle.getCurrentModule());
+                        m_resolverState.refreshSystemBundleModule(m_extensionManager.getModule());
 // TODO: REFACTOR - Not clear why this is here. We should look at all of these steps more closely.
                         setBundleStateAndNotify(bundle, Bundle.RESOLVED);
                     }
-                    else if (bundle.isExtension())
+                    else if (wasExtension)
                     {
                         setBundleStateAndNotify(bundle, Bundle.INSTALLED);
                     }
@@ -1649,6 +1646,28 @@
                 {
                     setBundleStateAndNotify(bundle, Bundle.INSTALLED);
                 }
+                else
+                {
+                    // Acquire bundle lock.
+                    try
+                    {
+                        acquireBundleLock(this, Bundle.RESOLVED | Bundle.STARTING | Bundle.ACTIVE);
+                    }
+                    catch (IllegalStateException ex)
+                    {
+                        throw new BundleException(
+                            "System bundle must be active to attach an extension.");
+                    }
+
+                    try
+                    {
+                        m_extensionManager.startExtensionBundle(this, bundle);
+                    }
+                    finally
+                    {
+                        releaseBundleLock(this);
+                    }
+                }
 
                 fireBundleEvent(BundleEvent.UNRESOLVED, bundle);
 
@@ -1665,7 +1684,7 @@
                 {
                     try
                     {
-                        if (!bundle.isUsed())
+                        if (!bundle.isUsed() && !bundle.isExtension())
                         {
                             try
                             {
@@ -1891,6 +1910,8 @@
             if (bundle.isExtension())
             {
                 bundle.setPersistentStateUninstalled();
+                bundle.setRemovalPending(true);
+                rememberUninstalledBundle(bundle);
                 setBundleStateAndNotify(bundle, Bundle.INSTALLED);
                 return;
             }
@@ -2178,7 +2199,7 @@
                 // Acquire bundle lock.
                 try
                 {
-                    acquireBundleLock(this, Bundle.STARTING | Bundle.ACTIVE);
+                    acquireBundleLock(this, Bundle.RESOLVED | Bundle.STARTING | Bundle.ACTIVE);
                 }
                 catch (IllegalStateException ex)
                 {
@@ -3155,11 +3176,7 @@
             // has been updated or uninstalled.
             for (int i = 0; (bundles != null) && !restart && (i < bundles.length); i++)
             {
-                if (bundles[i].isExtension())
-                {
-                    restart = true;
-                }
-                else if (systemBundle == bundles[i])
+                if (systemBundle == bundles[i])
                 {
                     Bundle[] allBundles = getBundles();
                     for (int j = 0; !restart && j < allBundles.length; j++)
@@ -3182,6 +3199,7 @@
             // Remove any targeted bundles from the uninstalled bundles
             // array, since they will be removed from the system after
             // the refresh.
+            // TODO: FRAMEWORK - Is this correct?
             for (int i = 0; (bundles != null) && (i < bundles.length); i++)
             {
                 forgetUninstalledBundle(bundles[i]);
@@ -3197,13 +3215,12 @@
                 RefreshHelper[] helpers = new RefreshHelper[bundles.length];
                 for (int i = 0; i < bundles.length; i++)
                 {
-                    if (!bundles[i].isExtension())
-                    {
-                        helpers[i] = new RefreshHelper(bundles[i]);
-                    }
+                    helpers[i] = new RefreshHelper(bundles[i]);
                 }
 
                 // Stop, purge or remove, and reinitialize all bundles first.
+                // TODO: FRAMEWORK - this will stop the system bundle if
+                // somebody called refresh 0. Is this what we want?
                 for (int i = 0; i < helpers.length; i++)
                 {
                     if (helpers[i] != null)
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 aa72d91..62d55f0 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
@@ -67,8 +67,9 @@
     private final URLStreamHandler m_streamHandler;
 
     private final String m_manifestVersion;
-    private final Version m_version;
+    private final boolean m_isExtension;
     private final String m_symbolicName;
+    private final Version m_version;
 
     private final ICapability[] m_capabilities;
     private final IRequirement[] m_requirements;
@@ -125,6 +126,7 @@
         m_bootPkgWildcards = bootPkgWildcards;
         m_manifestVersion = null;
         m_symbolicName = null;
+        m_isExtension = false;
         m_version = null;
         m_capabilities = null;
         m_requirements = null;
@@ -157,12 +159,12 @@
         // system bundle directly later on.
         m_manifestVersion = mp.getManifestVersion();
         m_version = mp.getBundleVersion();
-        m_capabilities = (Util.isExtensionBundle(m_headerMap))
-            ? null : mp.getCapabilities();
+        m_capabilities = mp.isExtension() ? null : mp.getCapabilities();
         m_requirements = mp.getRequirements();
         m_dynamicRequirements = mp.getDynamicRequirements();
         m_nativeLibraries = mp.getLibraries();
         m_symbolicName = mp.getSymbolicName();
+        m_isExtension = mp.isExtension();
 
         // Verify that all native libraries exist in advance; this will
         // throw an exception if the native library does not exist.
@@ -203,6 +205,11 @@
         return m_headerMap;
     }
 
+    public boolean isExtension()
+    {
+        return m_isExtension;
+    }
+
     public String getSymbolicName()
     {
         return m_symbolicName;
diff --git a/framework/src/main/java/org/apache/felix/framework/util/Util.java b/framework/src/main/java/org/apache/felix/framework/util/Util.java
index 1372157..685ddba 100644
--- a/framework/src/main/java/org/apache/felix/framework/util/Util.java
+++ b/framework/src/main/java/org/apache/felix/framework/util/Util.java
@@ -27,7 +27,6 @@
 import java.util.Properties;
 
 import org.apache.felix.framework.util.manifestparser.Capability;
-import org.apache.felix.framework.util.manifestparser.ManifestParser;
 import org.apache.felix.moduleloader.*;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.Constants;
@@ -36,15 +35,6 @@
 public class Util
 {
     /**
-     * Check whether the given manifest headers are from an extension bundle.
-     */
-    public static boolean isExtensionBundle(Map headers)
-    {
-        return (ManifestParser.parseExtensionBundleHeader((String)
-                headers.get(Constants.FRAGMENT_HOST)) != null);
-    }
-
-    /**
      * Converts a module identifier to a bundle identifier. Module IDs
      * are typically <tt>&lt;bundle-id&gt;.&lt;revision&gt;</tt>; this
      * method returns only the portion corresponding to the bundle ID.
diff --git a/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java b/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java
index 4b4934e..d8e8e38 100644
--- a/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java
+++ b/framework/src/main/java/org/apache/felix/framework/util/manifestparser/ManifestParser.java
@@ -29,16 +29,17 @@
 
 public class ManifestParser
 {
-    private Logger m_logger;
-    private Map m_configMap;
-    private Map m_headerMap;
-    private String m_bundleSymbolicName;
-    private Version m_bundleVersion;
-    private ICapability[] m_capabilities;
-    private IRequirement[] m_requirements;
-    private IRequirement[] m_dynamicRequirements;
-    private R4LibraryClause[] m_libraryHeaders;
-    private boolean m_libraryHeadersOptional = false;
+    private final Logger m_logger;
+    private final Map m_configMap;
+    private final Map m_headerMap;
+    private volatile boolean m_isExtension = false;
+    private volatile String m_bundleSymbolicName;
+    private volatile Version m_bundleVersion;
+    private volatile ICapability[] m_capabilities;
+    private volatile IRequirement[] m_requirements;
+    private volatile IRequirement[] m_dynamicRequirements;
+    private volatile R4LibraryClause[] m_libraryHeaders;
+    private volatile boolean m_libraryHeadersOptional = false;
 
     public ManifestParser(Logger logger, Map configMap, Map headerMap)
         throws BundleException
@@ -300,6 +301,11 @@
         return (manifestVersion == null) ? "1" : manifestVersion.trim();
     }
 
+    public boolean isExtension()
+    {
+        return m_isExtension;
+    }
+
     public String getSymbolicName()
     {
         return m_bundleSymbolicName;
@@ -785,7 +791,7 @@
 
         R4Directive extension = parseExtensionBundleHeader((String)
             m_headerMap.get(Constants.FRAGMENT_HOST));
-        
+
         if (extension != null)
         {
             if (!(Constants.EXTENSION_FRAMEWORK.equals(extension.getValue()) || 
@@ -795,6 +801,7 @@
                     "Extension bundle must have either 'extension:=framework' or 'extension:=bootclasspath'");
             }
             checkExtensionBundle();
+            m_isExtension = true;
         }
     }
 
@@ -806,7 +813,7 @@
             m_headerMap.containsKey(Constants.DYNAMICIMPORT_PACKAGE) ||
             m_headerMap.containsKey(Constants.BUNDLE_ACTIVATOR))
         {
-            throw new BundleException("Invalid Extension Bundle Manifest");
+            throw new BundleException("Invalid extension bundle manifest");
         }
     }
 
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 ed1e897..37994de 100644
--- a/framework/src/main/java/org/apache/felix/moduleloader/IModule.java
+++ b/framework/src/main/java/org/apache/felix/moduleloader/IModule.java
@@ -34,6 +34,7 @@
 
     // Metadata access methods.
     Map getHeaders();
+    boolean isExtension();
     String getSymbolicName();
     Version getVersion();
     ICapability[] getCapabilities();