diff --git a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java index 50be16d782f..f47825263f0 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java @@ -1100,6 +1100,14 @@ private AppContext createApplication(final AppInfo appInfo, ClassLoader classLoa return appContext; } catch (final ValidationException | DeploymentException ve) { + // these are not wrapped, but the partially deployed application still has to be + // rolled back or its deployment ids stay registered and make the next app reusing + // one of them fail with a DuplicateDeploymentIdException (TOMEE-4655) + try { + destroyApplication(appInfo); + } catch (final Exception e1) { + logger.debug("createApplication.undeployFailed", e1, appInfo.path); + } throw ve; } catch (final Throwable t) { try { @@ -2406,7 +2414,11 @@ public void destroyApplication(final AppInfo appInfo) throws UndeployException { final String deploymentID = String.valueOf(deployment.getDeploymentID()); try { final Container container = deployment.getContainer(); - container.stop(deployment); + // a deployment rolled back before startEjbs never got a container, or + // already lost it to a previous undeploy + if (container != null) { + container.stop(deployment); + } } catch (final Throwable t) { undeployException.getCauses().add(new Exception("bean: " + deploymentID + ": " + t.getMessage(), t)); } @@ -2417,8 +2429,10 @@ public void destroyApplication(final AppInfo appInfo) throws UndeployException { final String deploymentID = String.valueOf(bean.getDeploymentID()); try { final Container container = bean.getContainer(); - container.undeploy(bean); - bean.setContainer(null); + if (container != null) { + container.undeploy(bean); + bean.setContainer(null); + } } catch (final Throwable t) { undeployException.getCauses().add(new Exception("bean: " + deploymentID + ": " + t.getMessage(), t)); } finally { diff --git a/container/openejb-core/src/main/java/org/apache/openejb/core/singleton/SingletonInstanceManager.java b/container/openejb-core/src/main/java/org/apache/openejb/core/singleton/SingletonInstanceManager.java index 8f9552af1b1..5f80a3e7666 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/core/singleton/SingletonInstanceManager.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/core/singleton/SingletonInstanceManager.java @@ -213,6 +213,13 @@ private Instance createInstance(final ThreadContext callContext, final BeanConte public void freeInstance(final ThreadContext callContext) { final BeanContext beanContext = callContext.getBeanContext(); final Data data = (Data) beanContext.getContainerData(); + + // Possible the bean was never deployed into the container, e.g. when a + // deployment fails before startEjbs and is rolled back again + if (data == null) { + return; + } + final Future instanceFuture = data.singleton.get(); // Possible the instance was never created diff --git a/container/openejb-core/src/main/java/org/apache/openejb/core/stateful/StatefulContainer.java b/container/openejb-core/src/main/java/org/apache/openejb/core/stateful/StatefulContainer.java index 3a328b37c95..8576f7973b5 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/core/stateful/StatefulContainer.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/core/stateful/StatefulContainer.java @@ -288,6 +288,12 @@ public void stop(final BeanContext beanContext) throws OpenEJBException { public synchronized void undeploy(final BeanContext beanContext) throws OpenEJBException { final Data data = (Data) beanContext.getContainerData(); + // Possible the bean was never deployed into the container, e.g. when a + // deployment fails before startEjbs and is rolled back again + if (data == null) { + return; + } + final MBeanServer server = LocalMBeanServer.get(); for (final ObjectName objectName : data.jmxNames) { try { diff --git a/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/FailedDeploymentIdCleanupTest.java b/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/FailedDeploymentIdCleanupTest.java new file mode 100644 index 00000000000..8e072c388bc --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/FailedDeploymentIdCleanupTest.java @@ -0,0 +1,194 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.openejb.assembler.classic; + +import jakarta.ejb.Singleton; +import jakarta.ejb.Stateful; +import jakarta.enterprise.inject.spi.DeploymentException; +import jakarta.inject.Inject; +import org.apache.openejb.AppContext; +import org.apache.openejb.BeanContext; +import org.apache.openejb.Container; +import org.apache.openejb.OpenEJB; +import org.apache.openejb.config.ConfigurationFactory; +import org.apache.openejb.config.EjbModule; +import org.apache.openejb.jee.Beans; +import org.apache.openejb.jee.EjbJar; +import org.apache.openejb.jee.SingletonBean; +import org.apache.openejb.jee.StatefulBean; +import org.apache.openejb.jee.oejb3.EjbDeployment; +import org.apache.openejb.jee.oejb3.OpenejbJar; +import org.apache.openejb.loader.SystemInstance; +import org.apache.openejb.spi.ContainerSystem; +import org.junit.After; +import org.junit.Test; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; + +/** + * TOMEE-4655: when a deployment fails, the deployment ids it already registered must be + * released again. Otherwise the next application reusing one of those ids fails with a + * DuplicateDeploymentIdException before any of its own checks get a chance to run. + */ +public class FailedDeploymentIdCleanupTest { + + private static final String DEPLOYMENT_ID = "TheSharedDeploymentId"; + + @After + public void tearDown() { + OpenEJB.destroy(); + } + + @Test + public void deploymentIdIsReusableAfterAFailedCdiDeployment() throws Exception { + final ConfigurationFactory config = new ConfigurationFactory(); + final Assembler assembler = new Assembler(); + + assembler.createTransactionManager(config.configureService(TransactionServiceInfo.class)); + assembler.createSecurityService(config.configureService(SecurityServiceInfo.class)); + + // this app fails while CDI is starting, i.e. after initEjbs already registered the id + // but before startEjbs deployed the beans into their containers + try { + assembler.createApplication(config.configureApplication(failingModule())); + fail("the deployment was expected to fail while starting CDI"); + } catch (final DeploymentException expected) { + // that is the point of the test: the CDI failure is not wrapped, and it is + // this branch of createApplication that has to roll the deployment back + } + + final ContainerSystem containerSystem = SystemInstance.get().getComponent(ContainerSystem.class); + assertNull("the failed deployment leaked its deployment id", + containerSystem.getBeanContext(DEPLOYMENT_ID)); + + // a later, perfectly valid app reusing the same id must deploy just fine + assembler.createApplication(config.configureApplication(workingModule())); + + assertNotNull("the deployment id could not be reused after a failed deployment", + containerSystem.getBeanContext(DEPLOYMENT_ID)); + } + + /** + * The rollback added for TOMEE-4655 stops and undeploys beans that startEjbs never + * deployed into their container, because a CDI bootstrap failure happens before + * startEjbs runs. EjbJarBuilder assigns the container at build time while the + * container data only appears in Container.deploy, so undeploy has to tolerate a bean + * it never saw instead of throwing a NullPointerException per bean and burying the + * real cause. + */ + @Test + public void undeployingABeanThatWasNeverDeployedIsQuiet() throws Exception { + final ConfigurationFactory config = new ConfigurationFactory(); + final Assembler assembler = new Assembler(); + + assembler.createTransactionManager(config.configureService(TransactionServiceInfo.class)); + assembler.createSecurityService(config.configureService(SecurityServiceInfo.class)); + assembler.createContainer(config.configureService(SingletonSessionContainerInfo.class)); + assembler.createContainer(config.configureService(StatefulSessionContainerInfo.class)); + + // deploy a real application so the BeanContexts are built exactly as they are in + // production, then put them back into the state a rollback before startEjbs sees: + // a container assigned by EjbJarBuilder, but no container data + final AppContext appContext = assembler.createApplication( + config.configureApplication(module("some-app", ASingleton.class, AStateful.class))); + + for (final BeanContext beanContext : appContext.getBeanContexts()) { + final Container container = beanContext.getContainer(); + container.undeploy(beanContext); + + beanContext.setContainer(container); + assertNull("this test only makes sense while the bean was never deployed", + beanContext.getContainerData()); + + container.stop(beanContext); + container.undeploy(beanContext); + } + } + + private EjbModule failingModule() { + // the unsatisfied injection point makes OWB fail the application start. The + // stateful and singleton beans alongside it are the ones the rollback then has to + // stop and undeploy without them ever having been deployed into a container. + return module("failing-app", BrokenSingleton.class, ASingleton.class, AStateful.class); + } + + private EjbModule workingModule() { + return module("working-app", WorkingSingleton.class); + } + + private EjbModule module(final String moduleId, final Class beanClass, final Class... others) { + final EjbJar ejbJar = new EjbJar(moduleId); + ejbJar.addEnterpriseBean(new SingletonBean(beanClass)); + + final OpenejbJar openejbJar = new OpenejbJar(); + final EjbDeployment deployment = new EjbDeployment(); + deployment.setEjbName(beanClass.getSimpleName()); + deployment.setDeploymentId(DEPLOYMENT_ID); + openejbJar.addEjbDeployment(deployment); + + for (final Class other : others) { + if (AStateful.class.equals(other)) { + ejbJar.addEnterpriseBean(new StatefulBean(other)); + } else { + ejbJar.addEnterpriseBean(new SingletonBean(other)); + } + + final EjbDeployment otherDeployment = new EjbDeployment(); + otherDeployment.setEjbName(other.getSimpleName()); + otherDeployment.setDeploymentId(moduleId + "/" + other.getSimpleName()); + openejbJar.addEjbDeployment(otherDeployment); + } + + final EjbModule module = new EjbModule(ejbJar, openejbJar); + module.setModuleId(moduleId); + module.setBeans(new Beans()); + return module; + } + + public interface NoImplementationAnywhere { + void doSomething(); + } + + @Singleton + public static class BrokenSingleton { + @Inject + private NoImplementationAnywhere unsatisfied; + } + + @Singleton + public static class ASingleton { + public String hello() { + return "hello"; + } + } + + @Stateful + public static class AStateful { + public String hello() { + return "hello"; + } + } + + @Singleton + public static class WorkingSingleton { + public String hello() { + return "hello"; + } + } +}