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.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1339925 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/DeploymentAdminImpl.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/DeploymentAdminImpl.java
index b099016..8790737 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/DeploymentAdminImpl.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/DeploymentAdminImpl.java
@@ -64,10 +64,10 @@
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 void stop() {
cancel();
}
@@ -213,11 +212,14 @@
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 @@
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 {
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/ExplodingOutputtingInputStream.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/ExplodingOutputtingInputStream.java
index 4f8bcf1..c644a42 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/ExplodingOutputtingInputStream.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/ExplodingOutputtingInputStream.java
@@ -51,6 +51,7 @@
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 @@
}
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 @@
}
}
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 @@
}
}
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 @@
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 @@
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 @@
}
finally {
if (reader != null) {
- try {
- reader.close();
- }
- catch (IOException e) {
- // Not much we can do
- }
+ reader.close();
}
}
}
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/FileDeploymentPackage.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/FileDeploymentPackage.java
index a369d04..8d5f4dc 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/FileDeploymentPackage.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/FileDeploymentPackage.java
@@ -20,6 +20,7 @@
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 @@
}
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();) {