You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by ke...@apache.org on 2006/04/13 15:08:09 UTC

svn commit: r393796 - in /geronimo/branches/1.1/modules: connector-builder/src/test/org/apache/geronimo/connector/deployment/ deploy-tool/src/java/org/apache/geronimo/deployment/ deployment/src/java/org/apache/geronimo/deployment/ j2ee-builder/src/java...

Author: kevan
Date: Thu Apr 13 06:08:05 2006
New Revision: 393796

URL: http://svn.apache.org/viewcvs?rev=393796&view=rev
Log:
Fix for potential serialization problems during deploy. Can't close the DeploymentContext until after it has been serialized...

Modified:
    geronimo/branches/1.1/modules/connector-builder/src/test/org/apache/geronimo/connector/deployment/ConnectorModuleBuilderTest.java
    geronimo/branches/1.1/modules/deploy-tool/src/java/org/apache/geronimo/deployment/PluginBootstrap.java
    geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/ConfigurationBuilder.java
    geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/Deployer.java
    geronimo/branches/1.1/modules/j2ee-builder/src/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java
    geronimo/branches/1.1/modules/j2ee-builder/src/test/org/apache/geronimo/j2ee/deployment/EARConfigBuilderTest.java
    geronimo/branches/1.1/modules/service-builder/src/java/org/apache/geronimo/deployment/service/ServiceConfigBuilder.java

Modified: geronimo/branches/1.1/modules/connector-builder/src/test/org/apache/geronimo/connector/deployment/ConnectorModuleBuilderTest.java
URL: http://svn.apache.org/viewcvs/geronimo/branches/1.1/modules/connector-builder/src/test/org/apache/geronimo/connector/deployment/ConnectorModuleBuilderTest.java?rev=393796&r1=393795&r2=393796&view=diff
==============================================================================
--- geronimo/branches/1.1/modules/connector-builder/src/test/org/apache/geronimo/connector/deployment/ConnectorModuleBuilderTest.java (original)
+++ geronimo/branches/1.1/modules/connector-builder/src/test/org/apache/geronimo/connector/deployment/ConnectorModuleBuilderTest.java Thu Apr 13 06:08:05 2006
@@ -166,12 +166,16 @@
                     new ConnectorModuleBuilder(defaultEnvironment, defaultMaxSize, defaultMinSize, defaultBlockingTimeoutMilliseconds, defaultidleTimeoutMinutes, defaultXATransactionCaching, defaultXAThreadCaching),
                     resourceReferenceBuilder, null, serviceReferenceBuilder, kernel);
             ConfigurationData configData = null;
+            DeploymentContext context = null;
             try {
                 File planFile = new File(basedir, "src/test-data/data/external-application-plan.xml");
                 Object plan = configBuilder.getDeploymentPlan(planFile, rarFile);
-                List configurations = configBuilder.buildConfiguration(false, plan, rarFile, Collections.singleton(configurationStore), configurationStore);
-                configData = (ConfigurationData) configurations.get(0);
+                context = configBuilder.buildConfiguration(false, plan, rarFile, Collections.singleton(configurationStore), configurationStore);
+                configData = context.getConfigurationData();
             } finally {
+                if (context != null) {
+                    context.close();
+                }
                 if (configData != null) {
                     DeploymentUtil.recursiveDelete(configData.getConfigurationDir());
                 }

Modified: geronimo/branches/1.1/modules/deploy-tool/src/java/org/apache/geronimo/deployment/PluginBootstrap.java
URL: http://svn.apache.org/viewcvs/geronimo/branches/1.1/modules/deploy-tool/src/java/org/apache/geronimo/deployment/PluginBootstrap.java?rev=393796&r1=393795&r2=393796&view=diff
==============================================================================
--- geronimo/branches/1.1/modules/deploy-tool/src/java/org/apache/geronimo/deployment/PluginBootstrap.java (original)
+++ geronimo/branches/1.1/modules/deploy-tool/src/java/org/apache/geronimo/deployment/PluginBootstrap.java Thu Apr 13 06:08:05 2006
@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.jar.JarOutputStream;
 
+import org.apache.geronimo.deployment.DeploymentContext;
 import org.apache.geronimo.deployment.service.ServiceConfigBuilder;
 import org.apache.geronimo.deployment.xbeans.ConfigurationDocument;
 import org.apache.geronimo.deployment.xbeans.ConfigurationType;
@@ -104,12 +105,17 @@
                 return null;
             }
         };
-        List configurations = builder.buildConfiguration(false, config, null, Collections.singleton(targetConfigurationStore), targetConfigurationStore);
-        ConfigurationData configurationData = (ConfigurationData) configurations.get(0);
+
+        DeploymentContext context = builder.buildConfiguration(false, config, null, Collections.singleton(targetConfigurationStore), targetConfigurationStore);
+        ConfigurationData configurationData = context.getConfigurationData();
 
         JarOutputStream out = new JarOutputStream(new FileOutputStream(carFile));
         ExecutableConfigurationUtil.writeConfiguration(configurationData, out);
         out.flush();
         out.close();
+
+        if (context != null) {
+            context.close();
+        }
     }
 }

Modified: geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/ConfigurationBuilder.java
URL: http://svn.apache.org/viewcvs/geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/ConfigurationBuilder.java?rev=393796&r1=393795&r2=393796&view=diff
==============================================================================
--- geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/ConfigurationBuilder.java (original)
+++ geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/ConfigurationBuilder.java Thu Apr 13 06:08:05 2006
@@ -24,6 +24,7 @@
 import java.util.jar.JarFile;
 
 import org.apache.geronimo.common.DeploymentException;
+import org.apache.geronimo.deployment.DeploymentContext;
 import org.apache.geronimo.kernel.config.ConfigurationStore;
 import org.apache.geronimo.kernel.repository.Artifact;
 
@@ -59,9 +60,9 @@
      * @param module the module to build
      * @param configurationStores
      * @param targetConfigurationStore
-     * @return the configuration datas created from the deployment
+     * @return the deployment context created from the deployment (the contexts must be closed by the caller)
      * @throws IOException if there was a problem reading or writing the files
      * @throws org.apache.geronimo.common.DeploymentException if there was a problem with the configuration
      */
-    List buildConfiguration(boolean inPlaceDeployment, Object plan, JarFile module, Collection configurationStores, ConfigurationStore targetConfigurationStore) throws IOException, DeploymentException;
+    DeploymentContext buildConfiguration(boolean inPlaceDeployment, Object plan, JarFile module, Collection configurationStores, ConfigurationStore targetConfigurationStore) throws IOException, DeploymentException;
 }

Modified: geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/Deployer.java
URL: http://svn.apache.org/viewcvs/geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/Deployer.java?rev=393796&r1=393795&r2=393796&view=diff
==============================================================================
--- geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/Deployer.java (original)
+++ geronimo/branches/1.1/modules/deployment/src/java/org/apache/geronimo/deployment/Deployer.java Thu Apr 13 06:08:05 2006
@@ -20,6 +20,7 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.geronimo.common.DeploymentException;
+import org.apache.geronimo.deployment.DeploymentContext;
 import org.apache.geronimo.deployment.util.DeploymentUtil;
 import org.apache.geronimo.gbean.GBeanInfo;
 import org.apache.geronimo.gbean.GBeanInfoBuilder;
@@ -280,9 +281,14 @@
                 throw new DeploymentException("No ConfigurationStores!");
             }
             ConfigurationStore store = (ConfigurationStore) stores.iterator().next();
-            List configurations = builder.buildConfiguration(inPlaceConfiguration, plan, module, stores, store);
+            // It's our responsibility to close this context, once we're done with it...
+            DeploymentContext context = builder.buildConfiguration(inPlaceConfiguration, plan, module, stores, store);
+            List configurations = new ArrayList();
+            configurations.add(context.getConfigurationData());
+            configurations.addAll(context.getAdditionalDeployment());
+
             if (configurations.isEmpty()) {
-                throw new DeploymentException("Deployer did not create any configuration");
+                throw new DeploymentException("Deployer did not create any configurations");
             }
             try {
                 if (targetFile != null) {
@@ -314,6 +320,10 @@
                 cleanupConfigurations(configurations);
                 // unlikely as we just built this
                 throw new DeploymentException(e);
+            } finally {
+                if (context != null) {
+                    context.close();
+                }
             }
         } catch(Throwable e) {
             //TODO not clear all errors will result in total cleanup

Modified: geronimo/branches/1.1/modules/j2ee-builder/src/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java
URL: http://svn.apache.org/viewcvs/geronimo/branches/1.1/modules/j2ee-builder/src/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java?rev=393796&r1=393795&r2=393796&view=diff
==============================================================================
--- geronimo/branches/1.1/modules/j2ee-builder/src/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java (original)
+++ geronimo/branches/1.1/modules/j2ee-builder/src/java/org/apache/geronimo/j2ee/deployment/EARConfigBuilder.java Thu Apr 13 06:08:05 2006
@@ -18,6 +18,7 @@
 
 import org.apache.geronimo.common.DeploymentException;
 import org.apache.geronimo.deployment.ConfigurationBuilder;
+import org.apache.geronimo.deployment.DeploymentContext;
 import org.apache.geronimo.deployment.service.EnvironmentBuilder;
 import org.apache.geronimo.deployment.service.ServiceConfigBuilder;
 import org.apache.geronimo.deployment.util.DeploymentUtil;
@@ -335,11 +336,11 @@
         return applicationInfo.getEnvironment().getConfigId();
     }
 
-    public List buildConfiguration(boolean inPlaceDeployment, Object plan, JarFile earFile, Collection configurationStores, ConfigurationStore targetConfigurationStore) throws IOException, DeploymentException {
+    public DeploymentContext buildConfiguration(boolean inPlaceDeployment, Object plan, JarFile earFile, Collection configurationStores, ConfigurationStore targetConfigurationStore) throws IOException, DeploymentException {
         assert plan != null;
         ApplicationInfo applicationInfo = (ApplicationInfo) plan;
 
-        EARContext earContext;
+        EARContext earContext = null;
         ConfigurationModuleType applicationType = applicationInfo.getType();
         Environment environment = applicationInfo.getEnvironment();
         Artifact configId = environment.getConfigId();
@@ -453,25 +454,37 @@
                 getBuilder(module).addGBeans(earContext, module, cl, repositories);
             }
 
-            List configurations = new ArrayList();
-            configurations.add(earContext.getConfigurationData());
-            configurations.addAll(earContext.getAdditionalDeployment());
-            earContext.close();
-            return configurations;
+            // it's the caller's responsibility to close the context...
+            return earContext;
         } catch (GBeanAlreadyExistsException e) {
             // todo delete owned configuraitons like appclients
+            if (earContext != null) {
+                earContext.close();
+            }
             DeploymentUtil.recursiveDelete(configurationDir);
             throw new DeploymentException(e);
         } catch (IOException e) {
+            if (earContext != null) {
+                earContext.close();
+            }
             DeploymentUtil.recursiveDelete(configurationDir);
             throw e;
         } catch (DeploymentException e) {
+            if (earContext != null) {
+                earContext.close();
+            }
             DeploymentUtil.recursiveDelete(configurationDir);
             throw e;
         } catch(RuntimeException e) {
+            if (earContext != null) {
+                earContext.close();
+            }
             DeploymentUtil.recursiveDelete(configurationDir);
             throw e;
         } catch(Error e) {
+            if (earContext != null) {
+                earContext.close();
+            }
             DeploymentUtil.recursiveDelete(configurationDir);
             throw e;
         } finally {

Modified: geronimo/branches/1.1/modules/j2ee-builder/src/test/org/apache/geronimo/j2ee/deployment/EARConfigBuilderTest.java
URL: http://svn.apache.org/viewcvs/geronimo/branches/1.1/modules/j2ee-builder/src/test/org/apache/geronimo/j2ee/deployment/EARConfigBuilderTest.java?rev=393796&r1=393795&r2=393796&view=diff
==============================================================================
--- geronimo/branches/1.1/modules/j2ee-builder/src/test/org/apache/geronimo/j2ee/deployment/EARConfigBuilderTest.java (original)
+++ geronimo/branches/1.1/modules/j2ee-builder/src/test/org/apache/geronimo/j2ee/deployment/EARConfigBuilderTest.java Thu Apr 13 06:08:05 2006
@@ -245,6 +245,7 @@
 
     public void testBuildConfiguration() throws Exception {
         ConfigurationData configurationData = null;
+        DeploymentContext context = null;
         try {
             EARConfigBuilder configBuilder = new EARConfigBuilder(defaultParentId,
                     transactionContextManagerAbstractNameQuery,
@@ -264,9 +265,12 @@
                     naming);
 
             Object plan = configBuilder.getDeploymentPlan(null, earFile);
-            List configurations = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
-            configurationData = (ConfigurationData) configurations.get(0);
+            context = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
+            configurationData = context.getConfigurationData();
         } finally {
+            if (context != null) {
+                context.close();
+            }
             if (configurationData != null) {
                 DeploymentUtil.recursiveDelete(configurationData.getConfigurationDir());
             }
@@ -292,16 +296,20 @@
                 naming);
 
         ConfigurationData configurationData = null;
+        DeploymentContext context = null;
         try {
             Object plan = configBuilder.getDeploymentPlan(new File(basedir, "target/plans/test-bad-ejb-jar.xml"), earFile);
-            List configurations = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
-            configurationData = (ConfigurationData) configurations.get(0);
+            context = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
+            configurationData = context.getConfigurationData();
             fail("Should have thrown a DeploymentException");
         } catch (DeploymentException e) {
             if (e.getCause() instanceof IOException) {
                 fail("Should not be complaining about bad vendor DD for invalid module entry");
             }
         } finally {
+            if (context != null) {
+                context.close();
+            }
             if (configurationData != null) {
                 DeploymentUtil.recursiveDelete(configurationData.getConfigurationDir());
             }
@@ -327,16 +335,20 @@
                 naming);
 
         ConfigurationData configurationData = null;
+        DeploymentContext context = null;
         try {
             Object plan = configBuilder.getDeploymentPlan(new File(basedir, "target/plans/test-bad-war.xml"), earFile);
-            List configurations = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
-            configurationData = (ConfigurationData) configurations.get(0);
+            context = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
+            configurationData = context.getConfigurationData();
             fail("Should have thrown a DeploymentException");
         } catch (DeploymentException e) {
             if (e.getCause() instanceof IOException) {
                 fail("Should not be complaining about bad vendor DD for invalid module entry");
             }
         } finally {
+            if (context != null) {
+                context.close();
+            }
             if (configurationData != null) {
                 DeploymentUtil.recursiveDelete(configurationData.getConfigurationDir());
             }
@@ -362,16 +374,20 @@
                 naming);
 
         ConfigurationData configurationData = null;
+        DeploymentContext context = null;
         try {
             Object plan = configBuilder.getDeploymentPlan(new File(basedir, "target/plans/test-bad-rar.xml"), earFile);
-            List configurations = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
-            configurationData = (ConfigurationData) configurations.get(0);
+            context = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
+            configurationData = context.getConfigurationData();
             fail("Should have thrown a DeploymentException");
         } catch (DeploymentException e) {
             if (e.getCause() instanceof IOException) {
                 fail("Should not be complaining about bad vendor DD for invalid module entry");
             }
         } finally {
+            if (context != null) {
+                context.close();
+            }
             if (configurationData != null) {
                 DeploymentUtil.recursiveDelete(configurationData.getConfigurationDir());
             }
@@ -397,16 +413,20 @@
                 naming);
 
         ConfigurationData configurationData = null;
+        DeploymentContext context = null;
         try {
             Object plan = configBuilder.getDeploymentPlan(new File(basedir, "target/plans/test-bad-car.xml"), earFile);
-            List configurations = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
-            configurationData = (ConfigurationData) configurations.get(0);
+            context = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
+            configurationData = context.getConfigurationData();
             fail("Should have thrown a DeploymentException");
         } catch (DeploymentException e) {
             if (e.getCause() instanceof IOException) {
                 fail("Should not be complaining about bad vendor DD for invalid module entry");
             }
         } finally {
+            if (context != null) {
+                context.close();
+            }
             if (configurationData != null) {
                 DeploymentUtil.recursiveDelete(configurationData.getConfigurationDir());
             }
@@ -433,14 +453,18 @@
 
 
         ConfigurationData configurationData = null;
+        DeploymentContext context = null;
         try {
             Object plan = configBuilder.getDeploymentPlan(null, earFile);
-            List configurations = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
-            configurationData = (ConfigurationData) configurations.get(0);
+            context = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
+            configurationData = context.getConfigurationData();
             fail("Should have thrown a DeploymentException");
         } catch (DeploymentException e) {
             // expected
         } finally {
+            if (context != null) {
+                context.close();
+            }
             if (configurationData != null) {
                 DeploymentUtil.recursiveDelete(configurationData.getConfigurationDir());
             }
@@ -466,14 +490,18 @@
                 naming);
 
         ConfigurationData configurationData = null;
+        DeploymentContext context = null;
         try {
             Object plan = configBuilder.getDeploymentPlan(null, earFile);
-            List configurations = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
-            configurationData = (ConfigurationData) configurations.get(0);
+            context = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
+            configurationData = context.getConfigurationData();
             fail("Should have thrown a DeploymentException");
         } catch (DeploymentException e) {
             // expected
         } finally {
+            if (context != null) {
+                context.close();
+            }
             if (configurationData != null) {
                 DeploymentUtil.recursiveDelete(configurationData.getConfigurationDir());
             }
@@ -499,14 +527,18 @@
                 naming);
 
         ConfigurationData configurationData = null;
+        DeploymentContext context = null;
         try {
             Object plan = configBuilder.getDeploymentPlan(null, earFile);
-            List configurations = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
-            configurationData = (ConfigurationData) configurations.get(0);
+            context = configBuilder.buildConfiguration(false, plan, earFile, Collections.singleton(configStore), configStore);
+            configurationData = context.getConfigurationData();
             fail("Should have thrown a DeploymentException");
         } catch (DeploymentException e) {
             // expected
         } finally {
+            if (context != null) {
+                context.close();
+            }
             if (configurationData != null) {
                 DeploymentUtil.recursiveDelete(configurationData.getConfigurationDir());
             }

Modified: geronimo/branches/1.1/modules/service-builder/src/java/org/apache/geronimo/deployment/service/ServiceConfigBuilder.java
URL: http://svn.apache.org/viewcvs/geronimo/branches/1.1/modules/service-builder/src/java/org/apache/geronimo/deployment/service/ServiceConfigBuilder.java?rev=393796&r1=393795&r2=393796&view=diff
==============================================================================
--- geronimo/branches/1.1/modules/service-builder/src/java/org/apache/geronimo/deployment/service/ServiceConfigBuilder.java (original)
+++ geronimo/branches/1.1/modules/service-builder/src/java/org/apache/geronimo/deployment/service/ServiceConfigBuilder.java Thu Apr 13 06:08:05 2006
@@ -157,13 +157,13 @@
         return environment.getConfigId();
     }
 
-    public List buildConfiguration(boolean inPlaceDeployment, Object plan, JarFile unused, Collection configurationStores, ConfigurationStore targetConfigurationStore) throws IOException, DeploymentException {
+    public DeploymentContext buildConfiguration(boolean inPlaceDeployment, Object plan, JarFile unused, Collection configurationStores, ConfigurationStore targetConfigurationStore) throws IOException, DeploymentException {
         ConfigurationType configType = (ConfigurationType) plan;
 
-        return Collections.singletonList(buildConfiguration(configType, configurationStores, targetConfigurationStore));
+        return buildConfiguration(configType, configurationStores, targetConfigurationStore);
     }
 
-    public ConfigurationData buildConfiguration(ConfigurationType configurationType, Collection configurationStores, ConfigurationStore targetConfigurationStore) throws DeploymentException, IOException {
+    public DeploymentContext buildConfiguration(ConfigurationType configurationType, Collection configurationStores, ConfigurationStore targetConfigurationStore) throws DeploymentException, IOException {
 
         Environment environment = EnvironmentBuilder.buildEnvironment(configurationType.getEnvironment(), defaultEnvironment);
         Artifact configId = environment.getConfigId();
@@ -173,18 +173,24 @@
         } catch (ConfigurationAlreadyExistsException e) {
             throw new DeploymentException(e);
         }
-        DeploymentContext context = new DeploymentContext(outfile, null, environment, ConfigurationModuleType.SERVICE, naming, repositories, configurationStores);
-        ClassLoader cl = context.getClassLoader();
 
-
-        AbstractName moduleName = naming.createRootName(configId, configId.toString(), NameFactory.SERVICE_MODULE);
-        GbeanType[] gbeans = configurationType.getGbeanArray();
+        DeploymentContext context = new DeploymentContext(outfile, null,environment, ConfigurationModuleType.SERVICE, naming, repositories, configurationStores);
         try {
+            ClassLoader cl = context.getClassLoader();
+
+
+            AbstractName moduleName = naming.createRootName(configId, configId.toString(), NameFactory.SERVICE_MODULE);
+            GbeanType[] gbeans = configurationType.getGbeanArray();
+
             addGBeans(gbeans, cl, moduleName, context);
-            return context.getConfigurationData();
-        } finally {
+            return context;
+        } catch (RuntimeException t) {
             context.close();
-        }
+            throw t;
+        } catch (Error e) {
+            context.close();
+            throw e;
+        } 
     }
 
     public static void addGBeans(GbeanType[] gbeans, ClassLoader cl, AbstractName moduleName, DeploymentContext context) throws DeploymentException {