Author: marrs
Date: Thu May 17 23:41:58 2012
New Revision: 1339925

URL: http://svn.apache.org/viewvc?rev=1339925&view=rev
Log:
Various fixes, the most important one was properly closing a stream that was 
reading a manifest in FileDeploymentPackage, to improve robustness when 
something goes wrong while manipulating files. The old code worked everywhere 
but on Windows, where a locked file prevented deletion, causing problems 
updating deployment packages. If you use Windows, you definitely need this fix.

Modified:
    
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/DeploymentAdminImpl.java
    
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/ExplodingOutputtingInputStream.java
    
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/FileDeploymentPackage.java

Modified: 
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/DeploymentAdminImpl.java
URL: 
http://svn.apache.org/viewvc/felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/DeploymentAdminImpl.java?rev=1339925&r1=1339924&r2=1339925&view=diff
==============================================================================
--- 
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/DeploymentAdminImpl.java
 (original)
+++ 
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/DeploymentAdminImpl.java
 Thu May 17 23:41:58 2012
@@ -64,10 +64,10 @@ public class DeploymentAdminImpl impleme
 
     private static final long TIMEOUT = 10000;
 
-    private BundleContext m_context; /* will be injected by dependencymanager 
*/
-    private PackageAdmin m_packageAdmin;    /* will be injected by 
dependencymanager */
-    private EventAdmin m_eventAdmin; /* will be injected by dependencymanager 
*/
-    private LogService m_log;        /* will be injected by dependencymanager 
*/
+    private volatile BundleContext m_context;       /* will be injected by 
dependencymanager */
+    private volatile PackageAdmin m_packageAdmin;   /* will be injected by 
dependencymanager */
+    private volatile EventAdmin m_eventAdmin;       /* will be injected by 
dependencymanager */
+    private volatile LogService m_log;              /* will be injected by 
dependencymanager */
     private DeploymentSessionImpl m_session = null;
     private final Map m_packages = new HashMap();
     private final List m_commandChain = new ArrayList();
@@ -116,7 +116,6 @@ public class DeploymentAdminImpl impleme
         }
     }
 
-
     public void stop() {
        cancel();
     }
@@ -213,11 +212,14 @@ public class DeploymentAdminImpl impleme
                 throw de;
             }
             try {
-                jarInput.close();
+                // note that calling close on this stream will wait until 
asynchronous processing
+                // is done
+                input.close();
             }
             catch (IOException e) {
-                // nothing we can do
-                m_log.log(LogService.LOG_WARNING, "Could not close stream 
properly", e);
+                succeeded = false;
+                m_log.log(LogService.LOG_ERROR, "Could not close stream 
properly", e);
+                throw new 
DeploymentException(DeploymentException.CODE_OTHER_ERROR, "Could not close 
stream properly", e);
             }
 
             File targetContents = m_context.getDataFile(PACKAGE_DIR + 
File.separator + source.getName() + File.separator + PACKAGECONTENTS_DIR);
@@ -231,10 +233,13 @@ public class DeploymentAdminImpl impleme
                     m_log.log(LogService.LOG_ERROR, "Could not merge source 
fix package with target deployment package", e);
                     throw new 
DeploymentException(DeploymentException.CODE_OTHER_ERROR, "Could not merge 
source fix package with target deployment package", e);
                 }
-            } else {
+            }
+            else {
                 File targetPackage = m_context.getDataFile(PACKAGE_DIR + 
File.separator + source.getName());
                 targetPackage.mkdirs();
-                ExplodingOutputtingInputStream.replace(targetPackage, 
tempPackage);
+                if (!ExplodingOutputtingInputStream.replace(targetPackage, 
tempPackage)) {
+                       throw new 
DeploymentException(DeploymentException.CODE_OTHER_ERROR, "Could not replace " 
+ targetPackage + " with " + tempPackage);
+                }
             }
             FileDeploymentPackage fileDeploymentPackage = null;
             try {

Modified: 
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/ExplodingOutputtingInputStream.java
URL: 
http://svn.apache.org/viewvc/felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/ExplodingOutputtingInputStream.java?rev=1339925&r1=1339924&r2=1339925&view=diff
==============================================================================
--- 
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/ExplodingOutputtingInputStream.java
 (original)
+++ 
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/ExplodingOutputtingInputStream.java
 Thu May 17 23:41:58 2012
@@ -51,6 +51,7 @@ class ExplodingOutputtingInputStream ext
     private final File m_contentDir;
     private final File m_indexFile;
     private final PipedInputStream m_input;
+    private Exception m_exception;
 
     /**
      * Creates an instance of this class.
@@ -65,8 +66,15 @@ class ExplodingOutputtingInputStream ext
     }
 
     public void close() throws IOException {
-        super.close();
-        waitFor();
+        try {
+            super.close();
+            if (m_exception != null) {
+                throw new IOException("Exception while processing the stream 
in the background: " + m_exception.getMessage(), m_exception);
+            }
+        }
+        finally {
+            waitFor();
+        }
     }
 
     private ExplodingOutputtingInputStream(InputStream inputStream, 
PipedOutputStream output, File index, File root) throws IOException {
@@ -121,12 +129,20 @@ class ExplodingOutputtingInputStream ext
             }
         }
         catch (IOException ex) {
-            // TODO: review the impact of this happening and how to react
+            pushException(ex);
         }
         finally {
             if (writer != null) {
                 writer.close();
             }
+            if (input != null) {
+                try {
+                    input.close();
+                }
+                catch (IOException e) {
+                    pushException(e);
+                }
+            }
         }
         
         try {
@@ -137,15 +153,21 @@ class ExplodingOutputtingInputStream ext
             }
         }
         catch (IOException e) {
-            e.printStackTrace();
+            pushException(e);
         }
     }
+    
+    private void pushException(Exception e) {
+        Exception e2 = new Exception(e.getMessage());
+        e2.setStackTrace(e.getStackTrace());
+        if (m_exception != null) {
+            e2.initCause(m_exception);
+        }
+        m_exception = e2;
+    }
 
     public static boolean replace(File target, File source) {
-        if (delete(target, true)) {
-            return rename(source, target);
-        }
-        return false;
+        return delete(target, true) && rename(source, target);
     }
     
     public static boolean copy(File from, File to) {
@@ -217,13 +239,20 @@ class ExplodingOutputtingInputStream ext
     private static boolean delete(File root, boolean deleteRoot) {
         boolean result = true;
         if (root.isDirectory()) {
-            File[] childs = root.listFiles();
-            for (int i = 0; i < childs.length; i++) {
-                result &= delete(childs[i], true);
+            File[] files = root.listFiles();
+            for (int i = 0; i < files.length; i++) {
+               if (files[i].isDirectory()) {
+                       result &= delete(files[i], true);
+               }
+               else {
+                       result &= files[i].delete();
+               }
             }
         }
         if (deleteRoot) {
-            result &= root.delete();
+               if (root.exists()) {
+                       result &= root.delete();
+               }
         }
         return result;
     }
@@ -274,7 +303,10 @@ class ExplodingOutputtingInputStream ext
 
         for (Iterator iter = targetFiles.iterator(); iter.hasNext();) {
             String path = (String) iter.next();
-            (new File(target, path)).delete();
+            File targetFile = new File(target, path);
+            if (!targetFile.delete()) {
+                throw new IOException("Could not delete " + targetFile);
+            }
         }
 
         GZIPOutputStream outputStream = new GZIPOutputStream(new 
FileOutputStream(new File(target, "META-INF/MANIFEST.MF")));
@@ -295,12 +327,7 @@ class ExplodingOutputtingInputStream ext
         }
         finally {
             if (reader != null) {
-                try {
-                    reader.close();
-                }
-                catch (IOException e) {
-                    // Not much we can do
-                }
+                reader.close();
             }
         }
     }

Modified: 
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/FileDeploymentPackage.java
URL: 
http://svn.apache.org/viewvc/felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/FileDeploymentPackage.java?rev=1339925&r1=1339924&r2=1339925&view=diff
==============================================================================
--- 
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/FileDeploymentPackage.java
 (original)
+++ 
felix/trunk/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/FileDeploymentPackage.java
 Thu May 17 23:41:58 2012
@@ -20,6 +20,7 @@ package org.apache.felix.deploymentadmin
 
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
@@ -52,11 +53,27 @@ public class FileDeploymentPackage exten
     }
 
     private FileDeploymentPackage(List index, File packageDir, BundleContext 
bundleContext, DeploymentAdminImpl deploymentAdmin) throws DeploymentException, 
IOException {
-        super(new Manifest(new GZIPInputStream(new FileInputStream(new 
File(packageDir, (String) index.remove(0))))), bundleContext, deploymentAdmin);
+        super(readManifest(index, packageDir), bundleContext, deploymentAdmin);
         m_index = index;
         m_contentsDir = packageDir;
     }
 
+    private static Manifest readManifest(List index, File packageDir) throws 
FileNotFoundException, IOException {
+        final File manifestFile = new File(packageDir, (String) 
index.remove(0));
+        InputStream is = null;
+        Manifest mf = null;
+        try {
+            is = new GZIPInputStream(new FileInputStream(manifestFile));
+            mf = new Manifest(is);
+        }
+        finally {
+            if (is != null) {
+                is.close();
+            }
+        }
+        return mf;
+    }
+
     public BundleInfoImpl[] getOrderedBundleInfos() {
         List result = new ArrayList();
         for(Iterator i = m_index.iterator(); i.hasNext();) {


Reply via email to