Various smaller improvements and fixes.

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1347608 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/AbstractDeploymentPackage.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/AbstractDeploymentPackage.java
index 4a9cd33..d8dbd33 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/AbstractDeploymentPackage.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/AbstractDeploymentPackage.java
@@ -41,9 +41,61 @@
  * deployment package data is obtained, this should be handled by extending classes.
  */
 public abstract class AbstractDeploymentPackage implements DeploymentPackage {
-    private static final String[] STRINGS = new String[] {};
-    private static final ResourceInfoImpl[] RESOURCE_INFO_IMPLS = new ResourceInfoImpl[] {};
-    private static final BundleInfoImpl[] BUNDLE_INFO_IMPLS = new BundleInfoImpl[] {};
+    /**
+     * Represents an empty deployment package.
+     */
+    private static final class EmptyDeploymentPackage extends AbstractDeploymentPackage {
+        private static final String[] STRINGS = new String[] {};
+        private static final ResourceInfoImpl[] RESOURCE_INFO_IMPLS = new ResourceInfoImpl[] {};
+        private static final BundleInfoImpl[] BUNDLE_INFO_IMPLS = new BundleInfoImpl[] {};
+
+        public String getHeader(String header) {
+            if (Constants.DEPLOYMENTPACKAGE_SYMBOLICMAME.equals(header)) { return ""; }
+            else if (Constants.DEPLOYMENTPACKAGE_VERSION.equals(header)) { return Version.emptyVersion.toString(); }
+            else { return null; }
+        }
+
+        public Bundle getBundle(String symbolicName) { return null; }
+
+        public BundleInfo[] getBundleInfos() { return BUNDLE_INFO_IMPLS; }
+
+        public BundleInfoImpl[] getBundleInfoImpls() { return BUNDLE_INFO_IMPLS; }
+
+        public ResourceInfoImpl[] getResourceInfos() { return RESOURCE_INFO_IMPLS; }
+
+        public String getName() { return ""; }
+
+        public String getResourceHeader(String resource, String header) { return null; }
+
+        public ServiceReference getResourceProcessor(String resource) { return null; }
+
+        public String[] getResources() { return STRINGS; }
+
+        public Version getVersion() { return Version.emptyVersion; }
+
+        public boolean isStale() { return true; }
+
+        public void uninstall() throws DeploymentException { throw new IllegalStateException("Can not uninstall stale DeploymentPackage"); }
+
+        public boolean uninstallForced() throws DeploymentException { throw new IllegalStateException("Can not uninstall stale DeploymentPackage"); }
+
+        public InputStream getBundleStream(String symbolicName) throws IOException { return null; }
+
+        public BundleInfoImpl[] getOrderedBundleInfos() { return BUNDLE_INFO_IMPLS; }
+
+        public ResourceInfoImpl[] getOrderedResourceInfos() { return RESOURCE_INFO_IMPLS; }
+
+        public InputStream getCurrentEntryStream() { throw new UnsupportedOperationException(); }
+
+        public AbstractInfo getNextEntry() throws IOException { throw new UnsupportedOperationException(); }
+
+        public String getDisplayName() { return ""; }
+
+        public URL getIcon() { return null; }
+    }
+
+    protected static final AbstractDeploymentPackage EMPTY_PACKAGE = new EmptyDeploymentPackage();
+
     private final BundleContext m_bundleContext;
     private final DeploymentAdminImpl m_deploymentAdmin;
     private final DeploymentPackageManifest m_manifest;
@@ -54,33 +106,7 @@
     private final String[] m_resourcePaths;
     private final boolean m_isFixPackage;
     private boolean m_isStale;
-    protected static final AbstractDeploymentPackage EMPTY_PACKAGE = new AbstractDeploymentPackage() {
-        public String getHeader(String header) {
-            if (Constants.DEPLOYMENTPACKAGE_SYMBOLICMAME.equals(header)) { return ""; }
-            else if (Constants.DEPLOYMENTPACKAGE_VERSION.equals(header)) { return Version.emptyVersion.toString(); }
-            else { return null; }
-        }
-        public Bundle getBundle(String symbolicName) { return null; }
-        public BundleInfo[] getBundleInfos() { return BUNDLE_INFO_IMPLS; }
-        public BundleInfoImpl[] getBundleInfoImpls() { return BUNDLE_INFO_IMPLS; }
-        public ResourceInfoImpl[] getResourceInfos() { return RESOURCE_INFO_IMPLS; }
-        public String getName() { return ""; }
-        public String getResourceHeader(String resource, String header) { return null; }
-        public ServiceReference getResourceProcessor(String resource) { return null; }
-        public String[] getResources() { return STRINGS; }
-        public Version getVersion() { return Version.emptyVersion; }
-        public boolean isStale() { return true; }
-        public void uninstall() throws DeploymentException { throw new IllegalStateException("Can not uninstall stale DeploymentPackage"); }
-        public boolean uninstallForced() throws DeploymentException { throw new IllegalStateException("Can not uninstall stale DeploymentPackage"); }
-        public InputStream getBundleStream(String symbolicName) throws IOException { return null; }
-        public BundleInfoImpl[] getOrderedBundleInfos() { return BUNDLE_INFO_IMPLS; }
-        public ResourceInfoImpl[] getOrderedResourceInfos() { return RESOURCE_INFO_IMPLS; }
-        public InputStream getCurrentEntryStream() { throw new UnsupportedOperationException(); }
-        public AbstractInfo getNextEntry() throws IOException { throw new UnsupportedOperationException(); }
-        public String getDisplayName() { return ""; }
-        public URL getIcon() { return null; }
-    };
-
+    
     /* Constructor only for use by the emptyPackage static variable */
     private AbstractDeploymentPackage() {
         m_bundleContext = null;
@@ -215,7 +241,6 @@
                     else {
                     	return null;
                     }
-                    
                 }
                 catch (InvalidSyntaxException e) {
                 	// TODO: log this
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 dc119e4..89a6cf6 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
@@ -68,7 +68,7 @@
     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 volatile DeploymentSessionImpl m_session = null;
     private final Map m_packages = new HashMap();
     private final List m_commandChain = new ArrayList();
     private final Semaphore m_semaphore = new Semaphore();
@@ -121,8 +121,9 @@
     }
 
     public boolean cancel() {
-        if (m_session != null) {
-            m_session.cancel();
+        DeploymentSessionImpl session = m_session;
+        if (session != null) {
+            session.cancel();
             return true;
         }
         return false;
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/Command.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/Command.java
index 3331027..8ecd9d8 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/Command.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/Command.java
@@ -48,9 +48,11 @@
      * Rolls back all actions that were added through the <code>addRollback(Runnable r)</code> method (in reverse
      * adding order). It is not guaranteed that the state of everything related to the command will be as if the
      * command was never executed, a best effort should be made though.
+     * 
+     * @param session the deployment session to rollback, should be used for logging purposes.
      */
-    public void rollback() {
-        for (ListIterator i = m_rollback.listIterator(m_commit.size()); i.hasPrevious();) {
+    protected void rollback(DeploymentSessionImpl session) {
+        for (ListIterator i = m_rollback.listIterator(m_rollback.size()); i.hasPrevious();) {
             Runnable runnable = (Runnable) i.previous();
             runnable.run();
         }
@@ -59,8 +61,10 @@
 
     /**
      * Commits all changes the command may have defined when it was executed by calling the <code>execute()</code> method.
+     * 
+     * @param session the deployment session to commit, should be used for logging purposes.
      */
-    protected void commit() {
+    protected void commit(DeploymentSessionImpl session) {
         for (ListIterator i = m_commit.listIterator(); i.hasNext();) {
             Runnable runnable = (Runnable) i.next();
             runnable.run();
@@ -110,5 +114,4 @@
     public void cancel() {
         m_cancelled = true;
     }
-
 }
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/CommitResourceCommand.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/CommitResourceCommand.java
index a70fed4..ec74568 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/CommitResourceCommand.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/CommitResourceCommand.java
@@ -31,7 +31,7 @@
 /**
  * Command that commits all the resource processors that were added to the command.
  */
-public class CommitResourceCommand extends Command implements Runnable {
+public class CommitResourceCommand extends Command {
 
     private final List m_processors = new ArrayList();
 
@@ -52,23 +52,25 @@
                 processor.commit();
             }
             catch (Exception e) {
+                // We cannot throw an exception, see OSGi spec.
                 session.getLog().log(LogService.LOG_ERROR, "Committing resource processor '" + processor + "' failed", e);
-                // TODO Throw exception?
             }
         }
         m_processors.clear();
     }
 
-    public void rollback() {
+    protected void rollback(DeploymentSessionImpl session) {
         for (ListIterator i = m_processors.listIterator(m_processors.size()); i.hasPrevious();) {
             ResourceProcessor processor = (ResourceProcessor) i.previous();
             try {
                 processor.rollback();
             }
             catch (Exception e) {
-                // TODO Log this?
+                // We cannot throw an exception, see OSGi spec.
+                session.getLog().log(LogService.LOG_ERROR, "Rollback of resource processor '" + processor + "' failed", e);
+            } finally {
+                i.remove();
             }
-            i.remove();
         }
     }
 
@@ -88,9 +90,4 @@
         m_processors.add(processor);
         return true;
     }
-
-    public void run() {
-        rollback();
-    }
-
 }
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/DeploymentSessionImpl.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/DeploymentSessionImpl.java
index f9d9c15..b435666 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/DeploymentSessionImpl.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/DeploymentSessionImpl.java
@@ -79,7 +79,7 @@
             }
         }
         for (Iterator i = m_commands.iterator(); i.hasNext();) {
-            ((Command) i.next()).commit();
+            ((Command) i.next()).commit(this);
         }
         m_currentCommand = null;
     }
@@ -87,7 +87,7 @@
     private void rollback(List executedCommands) {
         for (ListIterator i = executedCommands.listIterator(executedCommands.size()); i.hasPrevious();) {
             Command command = (Command) i.previous();
-            command.rollback();
+            command.rollback(this);
         }
     }
 
@@ -98,10 +98,13 @@
      */
     public boolean cancel() {
         m_cancelled = true;
-        if (m_currentCommand != null) {
-            m_currentCommand.cancel();
+
+        Command currentCommand = m_currentCommand;
+        if (currentCommand != null) {
+            currentCommand.cancel();
             return true;
         }
+
         return false;
     }
 
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/DropResourceCommand.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/DropResourceCommand.java
index fd54bf4..ae63cb8 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/DropResourceCommand.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/DropResourceCommand.java
@@ -41,10 +41,12 @@
      */
     public DropResourceCommand(CommitResourceCommand commitCommand) {
         m_commitCommand = commitCommand;
-        addRollback(m_commitCommand);
     }
 
     public void execute(DeploymentSessionImpl session) {
+        // Allow proper rollback in case the drop fails...
+        addRollback(new RollbackCommitAction(session));
+        
         AbstractDeploymentPackage target = session.getTargetAbstractDeploymentPackage();
         AbstractDeploymentPackage source = session.getSourceAbstractDeploymentPackage();
         BundleContext context = session.getBundleContext();
@@ -73,4 +75,16 @@
             }
         }
     }
+    
+    private class RollbackCommitAction implements Runnable {
+        private final DeploymentSessionImpl m_session; 
+        
+        public RollbackCommitAction(DeploymentSessionImpl session) {
+            m_session = session;
+        }
+        
+        public void run() {
+            m_commitCommand.rollback(m_session);
+        }
+    }
 }
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/ProcessResourceCommand.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/ProcessResourceCommand.java
index 55d56a0..eb34fc0 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/ProcessResourceCommand.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/ProcessResourceCommand.java
@@ -52,10 +52,12 @@
      */
     public ProcessResourceCommand(CommitResourceCommand commitCommand) {
         m_commitCommand = commitCommand;
-        addRollback(m_commitCommand);
     }
 
     public void execute(DeploymentSessionImpl session) throws DeploymentException {
+        // Allow proper rollback in case the drop fails...
+        addRollback(new RollbackCommitAction(session));
+
         AbstractDeploymentPackage source = session.getSourceAbstractDeploymentPackage();
         BundleContext context = session.getBundleContext();
 
@@ -69,6 +71,8 @@
         }
 
         try {
+            String allowForeignCustomerizers = System.getProperty("org.apache.felix.deploymentadmin.allowforeigncustomizers", "false");
+
         	while (!expectedResources.isEmpty()) {
             	AbstractInfo jarEntry = source.getNextEntry();
             	if (jarEntry == null) {
@@ -85,8 +89,7 @@
                 ServiceReference ref = source.getResourceProcessor(name);
                 if (ref != null) {
                     String serviceOwnerSymName = ref.getBundle().getSymbolicName();
-                    String allowForeignCustomerizers = System.getProperty("org.apache.felix.deploymentadmin.allowforeigncustomizers", "false");
-                    if (source.getBundleInfoByName(serviceOwnerSymName) != null || allowForeignCustomerizers.equals("true")) {
+                    if (source.getBundleInfoByName(serviceOwnerSymName) != null || "true".equals(allowForeignCustomerizers)) {
                         ResourceProcessor resourceProcessor = (ResourceProcessor) context.getService(ref);
                         if (resourceProcessor != null) {
                             try {
@@ -121,5 +124,16 @@
             throw new DeploymentException(DeploymentException.CODE_OTHER_ERROR, "Problem while reading stream", e);
         }
     }
-
+    
+    private class RollbackCommitAction implements Runnable {
+        private final DeploymentSessionImpl m_session; 
+        
+        public RollbackCommitAction(DeploymentSessionImpl session) {
+            m_session = session;
+        }
+        
+        public void run() {
+            m_commitCommand.rollback(m_session);
+        }
+    }
 }
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/SnapshotCommand.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/SnapshotCommand.java
index 920c042..2fde4ed 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/SnapshotCommand.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/SnapshotCommand.java
@@ -64,8 +64,8 @@
                     try {
                         snapshot.createNewFile();
                         store(root, snapshot);
-                        addRollback(new RestoreSnapshotRunnable(snapshot, root));
-                        addCommit(new DeleteSnapshotRunnable(snapshot));
+                        addRollback(new RestoreSnapshotRunnable(session, snapshot, root));
+                        addCommit(new DeleteSnapshotRunnable(session, snapshot));
                     }
                     catch (IOException e) {
                         snapshot.delete();
@@ -77,18 +77,6 @@
         }
     }
 
-    private void delete(File root, boolean deleteRoot) {
-        if (root.isDirectory()) {
-            File[] childs = root.listFiles();
-            for (int i = 0; i < childs.length; i++) {
-                delete(childs[i], true);
-            }
-        }
-        if (deleteRoot) {
-            root.delete();
-        }
-    }
-
     private void store(File source, File target) throws IOException {
         ZipOutputStream output = null;
         try {
@@ -142,68 +130,29 @@
         }
     }
 
-    private void unpack(File source, File target) throws IOException {
-        ZipInputStream input = null;
-        try {
-            input = new ZipInputStream(new FileInputStream(source));
-            for (ZipEntry entry = input.getNextEntry(); entry != null; entry = input.getNextEntry()) {
-                if (entry.isDirectory()) {
-                    (new File(target, entry.getName())).mkdirs();
-                }
-                else {
-                    OutputStream output = null;
-                    try {
-                        output = new FileOutputStream(target);
-                        byte[] buffer = new byte[4096];
-                        for (int i = input.read(buffer); i > -1; i = input.read(buffer)) {
-                            output.write(buffer, 0, i);
-                        }
-                    }
-                    finally {
-                        if (output != null) {
-                            try {
-                                output.close();
-                            }
-                            catch (Exception ex) {
-                                // Not much we can do
-                            }
-                        }
-                    }
-                }
-                input.closeEntry();
-            }
-        }
-        finally {
-            if (input != null) {
-                try {
-                    input.close();
-                }
-                catch (Exception ex) {
-                    // Not much we can do
-                }
-            }
-        }
-    }
-
-    class DeleteSnapshotRunnable implements Runnable {
-
+    private static class DeleteSnapshotRunnable implements Runnable {
+        private final DeploymentSessionImpl m_session;
         private final File m_snapshot;
 
-        private DeleteSnapshotRunnable(File snapshot) {
+        private DeleteSnapshotRunnable(DeploymentSessionImpl session, File snapshot) {
+            m_session = session;
             m_snapshot = snapshot;
         }
 
         public void run() {
-            m_snapshot.delete();
+            if (!m_snapshot.delete()) {
+                m_session.getLog().log(LogService.LOG_WARNING, "Failed to delete snapshot in " + m_snapshot + "!");
+            }
         }
     }
 
-    private class RestoreSnapshotRunnable implements Runnable {
-
+    private static class RestoreSnapshotRunnable implements Runnable {
+        private final DeploymentSessionImpl m_session;
         private final File m_snapshot;
         private final File m_root;
 
-        private RestoreSnapshotRunnable(File snapshot, File root) {
+        private RestoreSnapshotRunnable(DeploymentSessionImpl session, File snapshot, File root) {
+            m_session = session;
             m_snapshot = snapshot;
             m_root = root;
         }
@@ -214,11 +163,66 @@
                 unpack(m_snapshot, m_root);
             }
             catch (Exception ex) {
-                // TODO: log this
+                m_session.getLog().log(LogService.LOG_WARNING, "Failed to restore snapshot!", ex);
             }
             finally {
                 m_snapshot.delete();
             }
         }
-    }
+
+        private void delete(File root, boolean deleteRoot) {
+            if (root.isDirectory()) {
+                File[] childs = root.listFiles();
+                for (int i = 0; i < childs.length; i++) {
+                    delete(childs[i], true);
+                }
+            }
+            if (deleteRoot) {
+                root.delete();
+            }
+        }
+
+        private void unpack(File source, File target) throws IOException {
+            ZipInputStream input = null;
+            try {
+                input = new ZipInputStream(new FileInputStream(source));
+                for (ZipEntry entry = input.getNextEntry(); entry != null; entry = input.getNextEntry()) {
+                    if (entry.isDirectory()) {
+                        (new File(target, entry.getName())).mkdirs();
+                    }
+                    else {
+                        OutputStream output = null;
+                        try {
+                            output = new FileOutputStream(target);
+                            byte[] buffer = new byte[4096];
+                            for (int i = input.read(buffer); i > -1; i = input.read(buffer)) {
+                                output.write(buffer, 0, i);
+                            }
+                        }
+                        finally {
+                            if (output != null) {
+                                try {
+                                    output.close();
+                                }
+                                catch (Exception ex) {
+                                    // Not much we can do
+                                }
+                            }
+                        }
+                    }
+                    input.closeEntry();
+                }
+            }
+            finally {
+                if (input != null) {
+                    try {
+                        input.close();
+                    }
+                    catch (Exception ex) {
+                        // Not much we can do
+                    }
+                }
+            }
+        }
+   }
 }
\ No newline at end of file
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StartBundleCommand.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StartBundleCommand.java
index 8bc7056..202cde3 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StartBundleCommand.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StartBundleCommand.java
@@ -32,7 +32,6 @@
  */
 public class StartBundleCommand extends Command {
     private final RefreshPackagesMonitor m_refreshMonitor = new RefreshPackagesMonitor();
-    private static final int REFRESH_TIMEOUT = 10000;
 
     public void execute(DeploymentSessionImpl session) {
         AbstractDeploymentPackage source = session.getSourceAbstractDeploymentPackage();
@@ -81,8 +80,10 @@
     /**
      * Use this monitor when its desired to wait for the completion of the asynchronous PackageAdmin.refreshPackages() call.
      */
-    private class RefreshPackagesMonitor {
-        private boolean m_alreadyNotified = false;
+    private static class RefreshPackagesMonitor {
+        private static final int REFRESH_TIMEOUT = 10000;
+
+        private volatile boolean m_alreadyNotified = false;
 
         /**
          * Waits for the completion of the PackageAdmin.refreshPackages() call. Because
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StartCustomizerCommand.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StartCustomizerCommand.java
index c1079f2..9675663 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StartCustomizerCommand.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StartCustomizerCommand.java
@@ -27,6 +27,7 @@
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
 import org.osgi.service.deploymentadmin.DeploymentException;
+import org.osgi.service.log.LogService;
 
 /**
  * Command that starts all customizer bundles defined in the source deployment packages of a deployment
@@ -38,11 +39,13 @@
     public void execute(DeploymentSessionImpl session) throws DeploymentException {
         AbstractDeploymentPackage target = session.getTargetAbstractDeploymentPackage();
         AbstractDeploymentPackage source = session.getSourceAbstractDeploymentPackage();
+
         Set bundles = new HashSet();
         Set sourceBundlePaths = new HashSet();
+
         BundleInfoImpl[] targetInfos = target.getBundleInfoImpls();
         BundleInfoImpl[] sourceInfos = source.getBundleInfoImpls();
-        for(int i = 0; i < sourceInfos.length; i++) {
+        for (int i = 0; i < sourceInfos.length; i++) {
             if (sourceInfos[i].isCustomizer()) {
                 sourceBundlePaths.add(sourceInfos[i].getPath());
                 Bundle bundle = source.getBundle(sourceInfos[i].getSymbolicName());
@@ -51,7 +54,8 @@
                 }
             }
         }
-        for(int i = 0; i < targetInfos.length; i++) {
+
+        for (int i = 0; i < targetInfos.length; i++) {
             if (targetInfos[i].isCustomizer() && !sourceBundlePaths.contains(targetInfos[i].getPath())) {
                 Bundle bundle = target.getBundle(targetInfos[i].getSymbolicName());
                 if (bundle != null) {
@@ -59,7 +63,8 @@
                 }
             }
         }
-        for(Iterator i = bundles.iterator(); i.hasNext(); ) {
+
+        for (Iterator i = bundles.iterator(); i.hasNext();) {
             Bundle bundle = (Bundle) i.next();
             try {
                 bundle.start();
@@ -67,15 +72,16 @@
             catch (BundleException be) {
                 throw new DeploymentException(DeploymentException.CODE_OTHER_ERROR, "Could not start customizer bundle '" + bundle.getSymbolicName() + "'", be);
             }
-            addRollback(new StopCustomizerRunnable(bundle));
+            addRollback(new StopCustomizerRunnable(session, bundle));
         }
     }
 
     private static class StopCustomizerRunnable implements Runnable {
-
+        private final DeploymentSessionImpl m_session;
         private final Bundle m_bundle;
 
-        public StopCustomizerRunnable(Bundle bundle) {
+        public StopCustomizerRunnable(DeploymentSessionImpl session, Bundle bundle) {
+            m_session = session;
             m_bundle = bundle;
         }
 
@@ -84,10 +90,8 @@
                 m_bundle.stop();
             }
             catch (BundleException e) {
-                // TODO log this
-                e.printStackTrace();
+                m_session.getLog().log(LogService.LOG_WARNING, "Failed to stop bundle '" + m_bundle.getSymbolicName() + "'", e);
             }
         }
-
     }
 }
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StopBundleCommand.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StopBundleCommand.java
index 8500f75..ca1258d 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StopBundleCommand.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/StopBundleCommand.java
@@ -38,6 +38,8 @@
 public class StopBundleCommand extends Command {
 
     public void execute(DeploymentSessionImpl session) throws DeploymentException {
+        String stopUnaffectedBundle = System.getProperty("org.apache.felix.deploymentadmin.stopunaffectedbundle", "true");
+        
         AbstractDeploymentPackage target = session.getTargetAbstractDeploymentPackage();
         BundleInfo[] bundleInfos = target.getOrderedBundleInfos();
         for (int i = 0; i < bundleInfos.length; i++) {
@@ -47,11 +49,10 @@
             String symbolicName = bundleInfos[i].getSymbolicName();
 			Bundle bundle = target.getBundle(symbolicName);
             if (bundle != null) {
-            	String stopUnaffectedBundle = System.getProperty("org.apache.felix.deploymentadmin.stopunaffectedbundle", "true");
-        		if (stopUnaffectedBundle.equalsIgnoreCase("false") && omitBundleStop(session, symbolicName)) {
+        		if ("false".equalsIgnoreCase(stopUnaffectedBundle) && omitBundleStop(session, symbolicName)) {
         			continue;
         		}
-                addRollback(new StartBundleRunnable(bundle));
+                addRollback(new StartBundleRunnable(session, bundle));
                 try {
                     bundle.stop();
                 }
@@ -87,11 +88,12 @@
 		return result;
 	}
 
-	private class StartBundleRunnable implements Runnable {
-
+	private static class StartBundleRunnable implements Runnable {
+        private final DeploymentSessionImpl m_session;
         private final Bundle m_bundle;
 
-        public StartBundleRunnable(Bundle bundle) {
+        public StartBundleRunnable(DeploymentSessionImpl session, Bundle bundle) {
+            m_session = session;
             m_bundle = bundle;
         }
 
@@ -100,11 +102,9 @@
                 m_bundle.start();
             }
             catch (BundleException e) {
-                // TODO: log this
-                e.printStackTrace();
+                m_session.getLog().log(LogService.LOG_WARNING, "Failed to start bundle '" + m_bundle.getSymbolicName() + "'", e);
             }
         }
-
     }
 }
 
diff --git a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/UpdateCommand.java b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/UpdateCommand.java
index 5159d29..84133f3 100644
--- a/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/UpdateCommand.java
+++ b/deploymentadmin/deploymentadmin/src/main/java/org/apache/felix/deploymentadmin/spi/UpdateCommand.java
@@ -51,7 +51,7 @@
         AbstractInfo[] bundleInfos = (AbstractInfo[]) source.getBundleInfos();
         for (int i = 0; i < bundleInfos.length; i++) {
             AbstractInfo bundleInfo = bundleInfos[i];
-            if(!bundleInfo.isMissing()) {
+            if (!bundleInfo.isMissing()) {
                 expectedBundles.put(bundleInfo.getPath(), bundleInfo);
             }
         }
@@ -91,7 +91,7 @@
                     }
                     throw new DeploymentException(DeploymentException.CODE_OTHER_ERROR, "Could not install new bundle '" + name + "'", be);
                 }
-                if (!bundle.getSymbolicName().equals(bundleInfo.getSymbolicName()) || !Version.parseVersion((String)bundle.getHeaders().get(org.osgi.framework.Constants.BUNDLE_VERSION)).equals(bundleInfo.getVersion())) {
+                if (!bundle.getSymbolicName().equals(bundleInfo.getSymbolicName()) || !Version.parseVersion((String) bundle.getHeaders().get(org.osgi.framework.Constants.BUNDLE_VERSION)).equals(bundleInfo.getVersion())) {
                     throw new DeploymentException(DeploymentException.CODE_OTHER_ERROR, "Installed/updated bundle version and/or symbolicnames do not match what was installed/updated");
                 }
             }
@@ -102,7 +102,6 @@
     }
 
     private static class UninstallBundleRunnable implements Runnable {
-
         private final Bundle m_bundle;
         private final LogService m_log;
 
@@ -122,7 +121,6 @@
     }
 
     private static class UpdateBundleRunnable implements Runnable {
-
         private final AbstractDeploymentPackage m_targetPackage;
         private final Bundle m_bundle;
         private final LogService m_log;
@@ -137,15 +135,16 @@
             InputStream is = null;
             try {
                 is = m_targetPackage.getBundleStream(m_bundle.getSymbolicName());
-                if(is != null){
-                    m_bundle.update(is);                    
+                if (is != null) {
+                    m_bundle.update(is);
                 }
                 throw new Exception("Unable to get Inputstream for bundle " + m_bundle.getSymbolicName());
             }
             catch (Exception e) {
                 m_log.log(LogService.LOG_WARNING, "Could not rollback update of bundle '" + m_bundle.getSymbolicName() + "'", e);
-            } finally {
-                if(is != null){
+            }
+            finally {
+                if (is != null) {
                     try {
                         is.close();
                     }
@@ -185,5 +184,4 @@
             }
         }
     }
-
 }