Return-Path: Delivered-To: apmail-geronimo-scm-archive@www.apache.org Received: (qmail 34751 invoked from network); 13 Apr 2006 13:15:54 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur.apache.org with SMTP; 13 Apr 2006 13:15:54 -0000 Received: (qmail 37457 invoked by uid 500); 13 Apr 2006 13:15:51 -0000 Delivered-To: apmail-geronimo-scm-archive@geronimo.apache.org Received: (qmail 37314 invoked by uid 500); 13 Apr 2006 13:15:50 -0000 Mailing-List: contact scm-help@geronimo.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: List-Post: Reply-To: dev@geronimo.apache.org List-Id: Delivered-To: mailing list scm@geronimo.apache.org Received: (qmail 37301 invoked by uid 99); 13 Apr 2006 13:15:50 -0000 Received: from asf.osuosl.org (HELO asf.osuosl.org) (140.211.166.49) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 13 Apr 2006 06:15:49 -0700 X-ASF-Spam-Status: No, hits=-9.4 required=10.0 tests=ALL_TRUSTED,NO_REAL_NAME X-Spam-Check-By: apache.org Received: from [209.237.227.194] (HELO minotaur.apache.org) (209.237.227.194) by apache.org (qpsmtpd/0.29) with SMTP; Thu, 13 Apr 2006 06:15:48 -0700 Received: (qmail 33030 invoked by uid 65534); 13 Apr 2006 13:12:12 -0000 Message-ID: <20060413131212.32681.qmail@minotaur.apache.org> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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... Date: Thu, 13 Apr 2006 13:08:09 -0000 To: scm@geronimo.apache.org From: kevan@apache.org X-Mailer: svnmailer-1.0.7 X-Virus-Checked: Checked by ClamAV on apache.org X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N 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 {