You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by xu...@apache.org on 2011/08/11 12:16:51 UTC

svn commit: r1156562 - in /geronimo/server/trunk: framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/ framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/ framework/modules/geronimo-system/src...

Author: xuhaihong
Date: Thu Aug 11 10:16:51 2011
New Revision: 1156562

URL: http://svn.apache.org/viewvc?rev=1156562&view=rev
Log:
GERONIMO-6106 Clean up temp files created in the deployment process
a. Start the reaper thread on the server start up, it will clean up the temp files generated by FileUtils in the last run
b. Keep a temp file list in the DeploymentContext, and will be deleted after the deployment finished

Modified:
    geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/Deployer.java
    geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/DeploymentContext.java
    geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/FileUtils.java
    geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/UnpackedJarFile.java
    geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/RepositoryConfigurationStore.java
    geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java
    geronimo/server/trunk/plugins/j2ee/geronimo-j2ee-builder/src/main/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java

Modified: geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/Deployer.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/Deployer.java?rev=1156562&r1=1156561&r2=1156562&view=diff
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/Deployer.java (original)
+++ geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/Deployer.java Thu Aug 11 10:16:51 2011
@@ -72,6 +72,8 @@ public class Deployer implements GBeanLi
     private static final Logger log = LoggerFactory.getLogger(Deployer.class);
 
     private final int REAPER_INTERVAL = 60 * 1000;
+    public static final String CLEAN_UP_ON_START_KEY = "org.apache.geronimo.deployer.cleanupOnStart";
+    private final boolean CLEAN_UP_ON_START = System.getProperty(CLEAN_UP_ON_START_KEY) == null ? true : Boolean.getBoolean(CLEAN_UP_ON_START_KEY);
     private DeployerReaper reaper;
     private final String remoteDeployAddress;
     private final Collection builders;
@@ -275,15 +277,6 @@ public class Deployer implements GBeanLi
 //            if (targetFile != null) {
 //                targetFile.delete();
 //            }
-
-            //Clean Up the created deploymentContext, as some initial work might be done in the buildConfiguration invocation
-            if (context != null) {
-                try {
-                    context.close();
-                } catch (Exception ingore) {
-                }
-            }
-
             if (e instanceof Error) {
                 log.error("Deployment failed due to ", e);
                 throw (Error) e;
@@ -296,6 +289,14 @@ public class Deployer implements GBeanLi
             throw new Error(e);
         } finally {
             JarUtils.close(module);
+            if (context != null) {
+                List<File> deleteOnExitFiles = context.getDeleteOnExitFiles();
+                try {
+                    context.close();
+                } catch (Exception e) {
+                }
+                cleanUpTemporaryDirectories(deleteOnExitFiles);
+            }
         }
     }
 
@@ -333,7 +334,7 @@ public class Deployer implements GBeanLi
             boolean install,
             Manifest manifest,
             ConfigurationStore store,
-            DeploymentContext context) throws DeploymentException, IOException, Throwable {
+            DeploymentContext context) throws DeploymentException {
         List<ConfigurationData> configurationDatas = new ArrayList<ConfigurationData>();
 
         boolean configsCleanupRequired = false;
@@ -389,26 +390,12 @@ public class Deployer implements GBeanLi
         } catch (DeploymentException e) {
             configsCleanupRequired = true;
             throw e;
-        } catch (IOException e) {
-            configsCleanupRequired = true;
-            throw e;
-        } catch (InvalidConfigException e) {
-            configsCleanupRequired = true;
-            // unlikely as we just built this
-            throw new DeploymentException(e);
         } catch (Throwable e) {
             // Could get here if serialization of the configuration failed (GERONIMO-1996)
             configsCleanupRequired = true;
-            throw e;
+            throw new DeploymentException(e);
         } finally {
             thread.setContextClassLoader(oldCl);
-            context.close();
-            //Clean up the temporary directory, now the deployment process is different with the old strategy
-            //Due to the applications installed in the repository folder is of archived type,
-            //the deployed application will be extracted to a temporary folder for analysis.
-            if (context.getBaseDir() != null && !FileUtils.recursiveDelete(context.getBaseDir())) {
-                reaper.delete(context.getBaseDir().getAbsolutePath(), "delete");
-            }
             if (configsCleanupRequired) {
                 // We do this after context is closed so the module jar isn't open
                 cleanupConfigurations(configurationDatas);
@@ -482,6 +469,12 @@ public class Deployer implements GBeanLi
         }
     }
 
+    private void cleanUpTemporaryDirectories(List<File> temporaryDirectories) {
+        for (File temporaryDirectory : temporaryDirectories) {
+            reaper.delete(temporaryDirectory.getAbsolutePath(), "delete");
+        }
+    }
+
     private void notifyWatchers(List list) {
         Artifact[] arts = new Artifact[list.size()];
         for (int i = 0; i < list.size(); i++) {
@@ -497,9 +490,8 @@ public class Deployer implements GBeanLi
         }
     }
 
-    private void cleanupConfigurations(List configurations) {
-        for (Iterator iterator = configurations.iterator(); iterator.hasNext();) {
-            ConfigurationData configurationData = (ConfigurationData) iterator.next();
+    private void cleanupConfigurations(List<ConfigurationData> configurations) {
+        for (ConfigurationData configurationData : configurations) {
             File configurationDir = configurationData.getConfigurationDir();
             if (!FileUtils.recursiveDelete(configurationDir)) {
                 reaper.delete(configurationDir.getAbsolutePath(), "delete");
@@ -532,19 +524,15 @@ public class Deployer implements GBeanLi
 
         public DeployerReaper(int reaperInterval) {
             this.reaperInterval = reaperInterval;
+            this.thread = new Thread(this, "Geronimo Config Store Reaper");
+            this.thread.setDaemon(true);
+            this.thread.start();
         }
 
         public void delete(String dir, String type) {
             pendingDeletionIndex.setProperty(dir, type);
-            log.debug("Queued deployment directory to be reaped " + dir);
-            startThread();
-        }
-
-        private synchronized void startThread() {
-            if (this.thread == null) {
-                this.thread = new Thread(this, "Geronimo Config Store Reaper");
-                this.thread.setDaemon(true);
-                this.thread.start();
+            if (log.isDebugEnabled()) {
+                log.debug("Queued deployment directory to be reaped " + dir);
             }
         }
 
@@ -573,6 +561,9 @@ public class Deployer implements GBeanLi
          * Reap any temporary directories left behind by previous runs.
          */
         private void reapBacklog() {
+            if (!CLEAN_UP_ON_START) {
+                return;
+            }
             try {
                 File tempFile = File.createTempFile("geronimo-deployer", ".tmpdir");
                 File tempDir = tempFile.getParentFile();
@@ -584,7 +575,28 @@ public class Deployer implements GBeanLi
                 for(String dir: backlog) {
                     File deleteDir = new File(tempDir, dir);
                     FileUtils.recursiveDelete(deleteDir);
-                    log.debug("Reaped deployment directory from previous runs " + deleteDir);
+                    if (log.isDebugEnabled()) {
+                        log.debug("Reaped deployment directory from previous runs " + deleteDir);
+                    }
+                }
+            } catch (IOException ignored) {
+            }
+            try {
+                File tempFile = FileUtils.createTempFile();
+                File tempDir = tempFile.getParentFile();
+                tempFile.delete();
+                String[] backlog = tempDir.list(new FilenameFilter(){
+                    public boolean accept(File dir, String name) {
+                        File file = new File(dir, name);
+                        boolean validTempFile = name.startsWith(FileUtils.DEFAULT_TEMP_PREFIX) && ((file.isDirectory() && name.endsWith(FileUtils.DEFAULT_TEMP_DIRECTORY_SUFFIX)) || file.isFile());
+                        return validTempFile && (file.lastModified() < FileUtils.FILE_UTILS_INITIALIZATION_TIME_MILL);
+                    }});
+                for(String dir: backlog) {
+                    File deleteDir = new File(tempDir, dir);
+                    FileUtils.recursiveDelete(deleteDir);
+                    if (log.isDebugEnabled()) {
+                        log.debug("Reaped deployment directory from previous runs " + deleteDir);
+                    }
                 }
             } catch (IOException ignored) {
             }
@@ -606,7 +618,9 @@ public class Deployer implements GBeanLi
 
                 if (FileUtils.recursiveDelete(deleteDir)) {
                     pendingDeletionIndex.remove(dirName);
-                    log.debug("Reaped deployment directory " + deleteDir);
+                    if (log.isDebugEnabled()) {
+                        log.debug("Reaped deployment directory " + deleteDir);
+                    }
                 }
             }
         }

Modified: geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/DeploymentContext.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/DeploymentContext.java?rev=1156562&r1=1156561&r2=1156562&view=diff
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/DeploymentContext.java (original)
+++ geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/DeploymentContext.java Thu Aug 11 10:16:51 2011
@@ -31,6 +31,7 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -98,6 +99,10 @@ public class DeploymentContext {
     protected final LinkedHashSet<String> bundleClassPath = new LinkedHashSet<String>();
     protected final ConfigurationModuleType moduleType;
     protected final Environment environment;
+    //This temporary directories is used to hold all the folders created in the deployment process.
+    //Due to some folders are required until the target application is installed,  so for those creators (e.g. xxxBuilder) have no chance
+    //to delete them.
+    private List<File> deleteOnExitFiles;
     //This provides services such as loading more bundles, it is NOT for the configuration we are constructing here.
     //It should be a disposable nested framework so as to not pollute the main framework with stuff we load as deployment parents.
     private final BundleContext bundleContext;
@@ -681,6 +686,40 @@ public class DeploymentContext {
         return failures;
     }
 
+    public void deleteFileOnExit(File file) {
+        if (deleteOnExitFiles == null) {
+            deleteOnExitFiles = new LinkedList<File>();
+        }
+        deleteOnExitFiles.add(file);
+    }
+
+    public boolean deleteFileOnExit(URL fileURL) {
+        if (!"file".equals(fileURL.getProtocol())) {
+            if (log.isDebugEnabled()) {
+                log.debug("Only file protocol URL is suppor, fileURL = " + fileURL);
+            }
+            return false;
+        }
+        File file = null;
+        try {
+            file = new File(fileURL.toURI());
+        } catch (Exception e) {
+            file = new File(fileURL.getPath());
+        }
+        if (file.exists()) {
+            deleteFileOnExit(file);
+            return true;
+        }
+        if (log.isDebugEnabled()) {
+            log.debug("Could not locate the target file based on the fileURL " + fileURL);
+        }
+        return false;
+    }
+
+    public List<File> getDeleteOnExitFiles() {
+        return deleteOnExitFiles == null ? Collections.<File>emptyList() : deleteOnExitFiles;
+    }
+
     private String verifyReference(GBeanData gbean, String referenceName, ReferencePatterns referencePatterns, Configuration configuration) {
         GReferenceInfo referenceInfo = gbean.getGBeanInfo().getReference(referenceName);
 

Modified: geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/FileUtils.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/FileUtils.java?rev=1156562&r1=1156561&r2=1156562&view=diff
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/FileUtils.java (original)
+++ geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/FileUtils.java Thu Aug 11 10:16:51 2011
@@ -22,7 +22,6 @@ import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
-import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -41,6 +40,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.zip.ZipEntry;
 
@@ -54,6 +54,14 @@ public class FileUtils {
 
     private static final Logger logger = LoggerFactory.getLogger(FileUtils.class);
 
+    public static final long FILE_UTILS_INITIALIZATION_TIME_MILL = System.currentTimeMillis();
+
+    public static final String DEFAULT_TEMP_PREFIX = "geronimo-fileutils";
+
+    public static final String DEFAULT_TEMP_FILE_SUFFIX = ".tmpfile";
+
+    public static final String DEFAULT_TEMP_DIRECTORY_SUFFIX = ".tmpdir";
+
     public static void copyFile(File source, File destination) throws IOException {
         copyFile(source, destination, IOUtils.DEFAULT_COPY_BUFFER_SIZE);
     }
@@ -80,7 +88,7 @@ public class FileUtils {
 
     // be careful to clean up the temp directory
     public static File createTempDir() throws IOException {
-        File tempDir = File.createTempFile("geronimo-fileutils", ".tmpdir");
+        File tempDir = File.createTempFile(DEFAULT_TEMP_PREFIX, DEFAULT_TEMP_DIRECTORY_SUFFIX);
         tempDir.delete();
         tempDir.mkdirs();
         deleteOnExit(tempDir);
@@ -90,7 +98,7 @@ public class FileUtils {
     // be careful to clean up the temp file... we tell the vm to delete this on exit
     // but VMs can't be trusted to acutally delete the file
     public static File createTempFile() throws IOException {
-        File tempFile = File.createTempFile("geronimo-fileutils", ".tmpfile");
+        File tempFile = File.createTempFile(DEFAULT_TEMP_PREFIX, DEFAULT_TEMP_FILE_SUFFIX);
         tempFile.deleteOnExit();
         return tempFile;
     }
@@ -98,7 +106,7 @@ public class FileUtils {
     // be careful to clean up the temp file... we tell the vm to delete this on exit
     // but VMs can't be trusted to acutally delete the file
     public static File createTempFile(String extension) throws IOException {
-        File tempFile = File.createTempFile("geronimo-fileutils", extension == null ? ".tmpdir" : extension);
+        File tempFile = File.createTempFile(DEFAULT_TEMP_PREFIX, extension == null ? DEFAULT_TEMP_DIRECTORY_SUFFIX : extension);
         tempFile.deleteOnExit();
         return tempFile;
     }
@@ -274,9 +282,9 @@ public class FileUtils {
                     }
                 } else {
                     Set<URL> matches = new LinkedHashSet<URL>();
-                    Enumeration entries = jarFile.entries();
+                    Enumeration<JarEntry> entries = jarFile.entries();
                     while (entries.hasMoreElements()) {
-                        ZipEntry entry = (ZipEntry) entries.nextElement();
+                        ZipEntry entry = entries.nextElement();
                         String fileName = entry.getName();
                         if (SelectorUtils.matchPath(pattern, fileName)) {
                             URL url = new URL(baseURL, fileName);

Modified: geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/UnpackedJarFile.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/UnpackedJarFile.java?rev=1156562&r1=1156561&r2=1156562&view=diff
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/UnpackedJarFile.java (original)
+++ geronimo/server/trunk/framework/modules/geronimo-kernel/src/main/java/org/apache/geronimo/kernel/util/UnpackedJarFile.java Thu Aug 11 10:16:51 2011
@@ -61,13 +61,7 @@ public class UnpackedJarFile extends Jar
                     in = new FileInputStream(manifestFile);
                     manifest = new Manifest(in);
                 } finally {
-                    if (in != null) {
-                        try {
-                            in.close();
-                        } catch (IOException e) {
-                            // ignore
-                        }
-                    }
+                    IOUtils.close(in);
                 }
             }
             manifestLoaded = true;

Modified: geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/RepositoryConfigurationStore.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/RepositoryConfigurationStore.java?rev=1156562&r1=1156561&r2=1156562&view=diff
==============================================================================
--- geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/RepositoryConfigurationStore.java (original)
+++ geronimo/server/trunk/framework/modules/geronimo-system/src/main/java/org/apache/geronimo/system/configuration/RepositoryConfigurationStore.java Thu Aug 11 10:16:51 2011
@@ -29,7 +29,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 import java.util.SortedSet;
-import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
@@ -308,8 +307,8 @@ public class RepositoryConfigurationStor
                          out.write(buf, 0, count);
                  }
                  in.close();
-                 out.closeEntry();                 
-             }             
+                 out.closeEntry();
+             }
              input.close();
     	}
     }
@@ -340,10 +339,12 @@ public class RepositoryConfigurationStor
 
     public void install(ConfigurationData configurationData) throws IOException, InvalidConfigException {
         // determine the source file/dir
-        log.debug("Writing config: " + configurationData);
+        if (log.isDebugEnabled()) {
+            log.debug("Writing config: " + configurationData);
+        }
         File source = configurationData.getInPlaceConfigurationDir() == null ? configurationData.getConfigurationDir()
                 : configurationData.getInPlaceConfigurationDir();
-        
+
         if (!source.exists()) {
             throw new InvalidConfigException("Source does not exist " + source);
         } else if (!source.canRead()) {
@@ -358,14 +359,20 @@ public class RepositoryConfigurationStor
         File destination = repository.getLocation(configId);
         if (!source.equals(destination)) {
             if (source.isFile()) {
-                log.debug("copying packed bundle from " + source + " to destination " + destination);
+                if (log.isDebugEnabled()) {
+                    log.debug("copying packed bundle from " + source + " to destination " + destination);
+                }
                 repository.copyToRepository(source, configId, null);
             } else {
-                log.debug("Packing bundle from " + source + " to destination " + destination);
+                if (log.isDebugEnabled()) {
+                    log.debug("Packing bundle from " + source + " to destination " + destination);
+                }
                 JarUtils.jarDirectory(source, destination);
             }
         } else {
-            log.debug("Plugin is already in location " + source);
+            if (log.isDebugEnabled()) {
+                log.debug("Plugin is already in location " + source);
+            }
         }
         // if directory in the correct place -- noop
        /*

Modified: geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java?rev=1156562&r1=1156561&r2=1156562&view=diff
==============================================================================
--- geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java (original)
+++ geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java Thu Aug 11 10:16:51 2011
@@ -305,9 +305,11 @@ public class AppClientModuleBuilder impl
 
         String specDD = null;
         ApplicationClient appClient = null;
+        boolean cleanUpSpecDDUrlRequired = false;
         try {
             if (specDDUrl == null) {
                 specDDUrl = JarUtils.createJarURL(moduleFile, "META-INF/application-client.xml");
+                cleanUpSpecDDUrlRequired = true;
             }
 
             // read in the entire specDD as a string, we need this for getDeploymentDescriptor
@@ -451,6 +453,10 @@ public class AppClientModuleBuilder impl
         for (ModuleBuilderExtension mbe : moduleBuilderExtensions) {
             mbe.createModule(module, plan, moduleFile, targetPath, specDDUrl, clientEnvironment, null, earName, naming, idBuilder);
         }
+
+        if(cleanUpSpecDDUrlRequired && specDDUrl != null) {
+            JarUtils.deleteJarFileURL(moduleFile, specDDUrl);
+        }
         if (standAlone) {
             Map<JndiKey, Map<String, Object>> appJndiContext = Module.share(Module.APP, module.getJndiContext());
 
@@ -588,7 +594,7 @@ public class AppClientModuleBuilder impl
                     connectionTrackerObjectName,
                     corbaGBeanObjectName,
                     earContext);
-
+            earContext.deleteFileOnExit(tempDirectory);
             appClientModule.setEarContext(appClientDeploymentContext);
             appClientModule.setRootEarContext(earContext);
 
@@ -646,32 +652,42 @@ public class AppClientModuleBuilder impl
                     }
                 }
             }
+            for (Module connectorModule : appClientModule.getModules()) {
+                if (connectorModule instanceof ConnectorModule) {
+                    getConnectorModuleBuilder().installModule(connectorModule.getModuleFile(), appClientDeploymentContext, connectorModule, configurationStores, targetConfigurationStore, repositories);
+                }
+            }
+            for (ModuleBuilderExtension mbe : moduleBuilderExtensions) {
+                mbe.installModule(module.getModuleFile(), appClientDeploymentContext, module, configurationStores, targetConfigurationStore, repositories);
+            }
         } catch (DeploymentException e) {
+            closeAppClientContextOnException(appClientDeploymentContext);
             throw e;
         } catch (IOException e) {
+            closeAppClientContextOnException(appClientDeploymentContext);
             throw new DeploymentException(e);
         }
-        for (Module connectorModule : appClientModule.getModules()) {
-            if (connectorModule instanceof ConnectorModule) {
-                getConnectorModuleBuilder().installModule(connectorModule.getModuleFile(), appClientDeploymentContext, connectorModule, configurationStores, targetConfigurationStore, repositories);
-            }
-        }
-        for (ModuleBuilderExtension mbe : moduleBuilderExtensions) {
-            mbe.installModule(module.getModuleFile(), appClientDeploymentContext, module, configurationStores, targetConfigurationStore, repositories);
-        }
     }
 
     public void initContext(EARContext earContext, Module clientModule, Bundle bundle) throws DeploymentException {
-        AppClientModule appClientModule = ((AppClientModule) clientModule);
-        namingBuilders.buildEnvironment(appClientModule.getSpecDD(), appClientModule.getVendorDD(), clientModule.getEnvironment());
+        try {
+            AppClientModule appClientModule = ((AppClientModule) clientModule);
+            namingBuilders.buildEnvironment(appClientModule.getSpecDD(), appClientModule.getVendorDD(), clientModule.getEnvironment());
 
-        for (Module connectorModule : appClientModule.getModules()) {
-            if (connectorModule instanceof ConnectorModule) {
-                getConnectorModuleBuilder().initContext(appClientModule.getEarContext(), connectorModule, bundle);
+            for (Module connectorModule : appClientModule.getModules()) {
+                if (connectorModule instanceof ConnectorModule) {
+                    getConnectorModuleBuilder().initContext(appClientModule.getEarContext(), connectorModule, bundle);
+                }
             }
-        }
-        for (ModuleBuilderExtension mbe : moduleBuilderExtensions) {
-            mbe.initContext(earContext, clientModule, bundle);
+            for (ModuleBuilderExtension mbe : moduleBuilderExtensions) {
+                mbe.initContext(earContext, clientModule, bundle);
+            }
+        } catch (DeploymentException e) {
+            closeAppClientContextOnException(clientModule.getEarContext());
+            throw e;
+        } catch (Exception e) {
+            closeAppClientContextOnException(clientModule.getEarContext());
+            throw new DeploymentException(e);
         }
     }
 
@@ -876,8 +892,6 @@ public class AppClientModuleBuilder impl
             }
 
         } catch (Throwable e) {
-            File appClientDir = appClientDeploymentContext.getBaseDir();
-            cleanupAppClientDir(appClientDir);
             if (e instanceof Error) {
                 throw (Error) e;
             } else if (e instanceof DeploymentException) {
@@ -1022,6 +1036,23 @@ public class AppClientModuleBuilder impl
         }
     }
 
+    /**
+     * Close the application client context if any exception is thrown in the deployment process
+     * This method should only be called while any error occurs, as it will clean the temporary directory.
+     * In a successful deployment, the temporary directory will be used to package the final application to repository directory
+     * @param appClientContext
+     */
+    private void closeAppClientContextOnException(EARContext appClientContext) {
+        if (appClientContext == null) {
+            return;
+        }
+        try {
+            appClientContext.close();
+        } catch (Exception e) {
+        }
+        cleanupAppClientDir(appClientContext.getBaseDir());
+    }
+
     private boolean cleanupAppClientDir(File configurationDir) {
         LinkedList<String> cannotBeDeletedList = new LinkedList<String>();
 

Modified: geronimo/server/trunk/plugins/j2ee/geronimo-j2ee-builder/src/main/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java
URL: http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/j2ee/geronimo-j2ee-builder/src/main/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java?rev=1156562&r1=1156561&r2=1156562&view=diff
==============================================================================
--- geronimo/server/trunk/plugins/j2ee/geronimo-j2ee-builder/src/main/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java (original)
+++ geronimo/server/trunk/plugins/j2ee/geronimo-j2ee-builder/src/main/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java Thu Aug 11 10:16:51 2011
@@ -87,6 +87,7 @@ import org.apache.geronimo.kernel.reposi
 import org.apache.geronimo.kernel.repository.MissingDependencyException;
 import org.apache.geronimo.kernel.repository.Repository;
 import org.apache.geronimo.kernel.util.FileUtils;
+import org.apache.geronimo.kernel.util.IOUtils;
 import org.apache.geronimo.kernel.util.JarUtils;
 import org.apache.geronimo.kernel.util.NestedJarFile;
 import org.apache.geronimo.management.J2EEResource;
@@ -374,17 +375,17 @@ public class EARConfigBuilder implements
         String specDD;
         Application application = null;
         if (earFile != null) {
+            URL applicationXmlUrl = null;
             try {
-                URL applicationXmlUrl = JarUtils.createJarURL(earFile, "META-INF/application.xml");
+                applicationXmlUrl = JarUtils.createJarURL(earFile, "META-INF/application.xml");
                 specDD = JarUtils.readAll(applicationXmlUrl);
                 //we found something called application.xml in the right place, if we can't parse it it's an error
                 InputStream in = applicationXmlUrl.openStream();
                 try {
                     application = (Application) JaxbJavaee.unmarshalJavaee(Application.class, in);
                  } finally {
-                    in.close();
+                    IOUtils.close(in);
                 }
-
             } catch (ParserConfigurationException e) {
                 throw new DeploymentException("Could not parse application.xml", e);
              } catch (SAXException e) {
@@ -392,13 +393,16 @@ public class EARConfigBuilder implements
              } catch (JAXBException e) {
                 throw new DeploymentException("Could not parse application.xml", e);
             } catch (Exception e) {
-//                ee5 spec allows optional application.xml, continue with application == null
+                //ee5 spec allows optional application.xml, continue with application == null
                 if (!earFile.getName().endsWith(".ear")) {
                     return null;
                 }
                 application = new Application();
+            } finally {
+                if (applicationXmlUrl != null) {
+                    JarUtils.deleteJarFileURL(earFile, applicationXmlUrl);
+                }
             }
-
         }
 
         GerApplicationType gerApplication = null;
@@ -586,6 +590,7 @@ public class EARConfigBuilder implements
                     corbaGBeanObjectName,
                     new HashMap()
             );
+            earContext.deleteFileOnExit(tempDirectory);
             applicationInfo.setEarContext(earContext);
             applicationInfo.setRootEarContext(earContext);
             earContext.getGeneralData().put(EARContext.MODULE_LIST_KEY, applicationInfo.getModuleLocations());
@@ -782,23 +787,28 @@ public class EARConfigBuilder implements
             // it's the caller's responsibility to close the context...
             return earContext;
         } catch (GBeanAlreadyExistsException e) {
-            cleanupContext(earContext);
             throw new DeploymentException(e);
         } catch (IOException e) {
-            cleanupContext(earContext);
             throw e;
         } catch (DeploymentException e) {
-            cleanupContext(earContext);
             throw e;
         } catch (RuntimeException e) {
-            cleanupContext(earContext);
             throw e;
         } catch (Error e) {
-            cleanupContext(earContext);
             throw e;
         } finally {
-            for (Module<?,?> module : applicationInfo.getModules()) {
+            for (Module<?, ?> module : applicationInfo.getModules()) {
                 module.close();
+                //In the non in-place deployment, a temporary file for each module in the ear will be created
+                if (!inPlaceDeployment) {
+                    try {
+                        FileUtils.recursiveDelete(new File(module.getModuleFile().getName()));
+                    } catch (Exception e) {
+                         if(log.isDebugEnabled()) {
+                             log.debug("Unable to delete the temporary file for module " + module.getName());
+                         }
+                    }
+                }
             }
         }
     }
@@ -836,21 +846,6 @@ public class EARConfigBuilder implements
         }
     }
 
-    private void cleanupContext(EARContext earContext) {
-        if (earContext == null) {
-            return;
-        }
-        File tempDirectory = earContext.getBaseDir();
-        try {
-            earContext.close();
-        } catch (Exception e) {
-        }
-        try {
-            cleanupConfigurationDir(tempDirectory);
-        } catch (Exception e) {
-        }
-    }
-
     private boolean cleanupConfigurationDir(File configurationDir) {
         LinkedList<String> cannotBeDeletedList = new LinkedList<String>();