Moved resolver state handling to the bundle so we can guarantee state changes
occur when operating on a bundle. (FELIX-851)


git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@737837 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 457089b..d9303e7 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
@@ -85,10 +85,26 @@
         return m_felix;
     }
 
-    synchronized void reset() throws Exception
+    synchronized void refresh(FelixResolverState state) throws Exception
     {
+        // We are refreshing the bundle. First, we must remove all existing
+        // modules from the resolver state and close them. Closing the modules
+        // is important, since we will likely be deleting their associated files.
+        for (int i = 0; i < m_modules.length; i++)
+        {
+            state.removeModule(m_modules[i]);
+            ((ModuleImpl) m_modules[i]).close();
+        }
+
+        // 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];
-        addModule(createModule());
+        final IModule module = createModule();
+        addModule(module);
+        state.addModule(module);
         m_state = Bundle.INSTALLED;
         m_stale = false;
         m_cachedHeaders.clear();
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 9b819a1..0c931fb 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -2987,8 +2987,7 @@
                 if (helpers[i] != null)
                 {
                     helpers[i].stop();
-                    helpers[i].purgeOrRemove();
-                    helpers[i].reinitialize();
+                    helpers[i].refreshOrRemove();
                 }
             }
 
@@ -3090,33 +3089,23 @@
         return activator;
     }
 
-    private void purgeBundle(BundleImpl bundle) throws Exception
+    private void refreshBundle(BundleImpl bundle) throws Exception
     {
         // Acquire bundle lock.
         acquireBundleLock(bundle);
 
         try
         {
-            // In case of a refresh, then we want to physically
-            // remove the bundle's modules from the module manager.
-            // This is necessary for two reasons: 1) because
-            // under Windows we won't be able to delete the bundle
-            // because files might be left open in the resource
-            // sources of its modules and 2) we want to make sure
-            // that no references to old modules exist since they
-            // will all be stale after the refresh. The only other
-            // way to do this is to remove the bundle, but that
-            // would be incorrect, because this is a refresh operation
-            // and should not trigger bundle REMOVE events.
-            IModule[] modules = bundle.getModules();
-// TODO: REFACTOR - It kind of sucks we need to remember this steps.
-            for (int i = 0; i < modules.length; i++)
+            try
             {
-                m_resolverState.removeModule(modules[i]);
+                // Reset the bundle object and fire UNRESOLVED event.
+                ((BundleImpl) bundle).refresh(m_resolverState);
+                fireBundleEvent(BundleEvent.UNRESOLVED, bundle);
             }
-
-            // Purge all bundle revisions, but the current one.
-            m_cache.getArchive(bundle.getBundleId()).purge();
+            catch (Exception ex)
+            {
+                fireFrameworkEvent(FrameworkEvent.ERROR, bundle, ex);
+            }
         }
         finally
         {
@@ -3639,7 +3628,7 @@
                 {
                     try
                     {
-                        purgeBundle(bundle);
+                        refreshBundle(bundle);
                     }
                     catch (Exception ex)
                     {
@@ -3732,7 +3721,7 @@
             }
         }
 
-        public void purgeOrRemove()
+        public void refreshOrRemove()
         {
             try
             {
@@ -3750,10 +3739,10 @@
                 }
                 else
                 {
-                    // This physically removes all old revisions of the
-                    // bundle from memory and only maintains the newest
-                    // version in the bundle cache.
-                    purgeBundle(m_bundle);
+                    // This physically removes all old bundle modules from
+                    // from memory and all old revisions from disk. It only
+                    // maintains the newest version in the bundle cache.
+                    refreshBundle(m_bundle);
                 }
             }
             catch (Exception ex)
@@ -3762,27 +3751,6 @@
             }
         }
 
-        public void reinitialize()
-        {
-            if (m_bundle != null)
-            {
-                try
-                {
-                    BundleImpl oldImpl = (BundleImpl) m_bundle;
-                    // Create the module for the new revision.
-                    oldImpl.reset();
-                    // Add new module to resolver state.
-// TODO: REFACTOR - It is not clean to have to remember these steps.
-                    m_resolverState.addModule(oldImpl.getCurrentModule());
-                    fireBundleEvent(BundleEvent.UNRESOLVED, m_bundle);
-                }
-                catch (Exception ex)
-                {
-                    fireFrameworkEvent(FrameworkEvent.ERROR, m_bundle, ex);
-                }
-            }
-        }
-
         public void restart()
         {
             if ((m_bundle != null) && (m_oldState == Bundle.ACTIVE))
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 42ce3ee..f70b147 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
@@ -130,23 +130,32 @@
 
             // Verify that all native libraries exist in advance; this will
             // throw an exception if the native library does not exist.
-            for (int i = 0; (m_nativeLibraries != null) && (i < m_nativeLibraries.length); i++)
+            try
             {
-                String entryName = m_nativeLibraries[i].getEntryName();
-                if (entryName != null)
+                for (int i = 0;
+                    (m_nativeLibraries != null) && (i < m_nativeLibraries.length);
+                    i++)
                 {
-                    if (m_content.getEntryAsNativeLibrary(entryName) == null)
+                    String entryName = m_nativeLibraries[i].getEntryName();
+                    if (entryName != null)
                     {
-                        throw new BundleException("Native library does not exist: " + entryName);
-// TODO: REFACTOR - Do we still have a memory leak here?
-//                  We have a memory leak here since we added a module above
-//                  and then don't remove it in case of an error; this may also
-//                  be a general issue for installing/updating bundles, so check.
-//                  This will likely go away when we refactor out the module
-//                  factory, but we will track it under FELIX-835 until then.
+                        if (m_content.getEntryAsNativeLibrary(entryName) == null)
+                        {
+                            throw new BundleException("Native library does not exist: " + entryName);
+                        }
                     }
                 }
             }
+            finally
+            {
+                // We close the module content here to make sure it is closed
+                // to avoid having to close it if there is an exception during
+                // the entire module creation process.
+// TODO: REFACTOR - If we do the above check here, then we open the module's content
+//       immediately every time, which means we must close it here so we don't have
+//       to remember to close it if there are other failures during module init.
+                m_content.close();
+            }
         }
         else
         {