Hold global lock while updating a bundle and performing a security check
and make sure we rollback the module if the security check fails. (FELIX-2802)


git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1063475 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 e3b525c..c41ab55 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
@@ -1090,6 +1090,14 @@
 
     synchronized boolean rollbackRevise() throws Exception
     {
+        boolean isExtension = isExtension();
+        Module m = m_modules.remove(m_modules.size() - 1);
+        if (!isExtension)
+        {
+            // Since revising a module adds the module to the global
+            // state, we must remove it from the global state on rollback.
+            getFramework().getResolverState().removeModule(m);
+        }
         return m_archive.rollbackRevise();
     }
 
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 1750efd..33d3c2d 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -1951,59 +1951,58 @@
                     throw new BundleException(
                         "Cannot acquire global lock to update the bundle.");
                 }
-                boolean wasExtension = bundle.isExtension();
                 try
                 {
-// TODO: REFACTOR - This adds the module to the resolver state, but should we do the
-//            security check first?
+                    // Try to revise.
+                    boolean wasExtension = bundle.isExtension();
                     bundle.revise(updateLocation, is);
+
+                    // Verify bundle revision.
+                    try
+                    {
+                        Object sm = System.getSecurityManager();
+
+                        if (sm != null)
+                        {
+                            ((SecurityManager) sm).checkPermission(
+                                new AdminPermission(bundle, AdminPermission.LIFECYCLE));
+                        }
+
+                        // If this is an update from a normal to an extension bundle
+                        // 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(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 (wasExtension)
+                        {
+                            setBundleStateAndNotify(bundle, Bundle.INSTALLED);
+                        }
+                    }
+                    catch (Throwable ex)
+                    {
+                        try
+                        {
+                            bundle.rollbackRevise();
+                        }
+                        catch (Exception busted)
+                        {
+                            m_logger.log(
+                                bundle, Logger.LOG_ERROR, "Unable to rollback.", busted);
+                        }
+
+                        throw ex;
+                    }
                 }
                 finally
                 {
                     // Always release the global lock.
                     releaseGlobalLock();
                 }
-
-                // Verify updated bundle.
-                try
-                {
-                    Object sm = System.getSecurityManager();
-
-                    if (sm != null)
-                    {
-                        ((SecurityManager) sm).checkPermission(
-                            new AdminPermission(bundle, AdminPermission.LIFECYCLE));
-                    }
-
-                    // If this is an update from a normal to an extension bundle
-                    // 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(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 (wasExtension)
-                    {
-                        setBundleStateAndNotify(bundle, Bundle.INSTALLED);
-                    }
-                }
-                catch (Throwable ex)
-                {
-                    try
-                    {
-                        bundle.rollbackRevise();
-                    }
-                    catch (Exception busted)
-                    {
-                        m_logger.log(
-                            bundle, Logger.LOG_ERROR, "Unable to rollback.", busted);
-                    }
-
-                    throw ex;
-                }
             }
             catch (Throwable ex)
             {