From d5405e8338563bcca50db87ec7164ff826a6bafd Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Thu, 23 Jul 2026 21:24:04 +0200 Subject: [PATCH 1/2] TOMEE-4655 - roll back a failed deployment's registered ids When a CDI or EJB deployment failed, Assembler.createApplication rethrew ValidationException/DeploymentException without undeploying the partially deployed application. CDI startup failures surface as a jakarta.enterprise.inject.spi.DeploymentException, so they took this path and left every already-registered BeanContext in the ContainerSystem. The next application reusing one of those deployment ids then failed with a DuplicateDeploymentIdException before reaching its own checks, turning one bad deployment into a cascade of unrelated failures. That catch clause only ever existed so these two exception types would not be wrapped in an OpenEJBException; skipping the rollback was an unintended side effect. Run destroyApplication(appInfo) on this path as well, then rethrow the original exception unchanged. Adds FailedDeploymentIdCleanupTest, which fails against the previous code with the exact "leaked its deployment id" symptom and passes with the fix. --- .../openejb/assembler/classic/Assembler.java | 8 ++ .../FailedDeploymentIdCleanupTest.java | 120 ++++++++++++++++++ 2 files changed, 128 insertions(+) create mode 100644 container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/FailedDeploymentIdCleanupTest.java 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..6967f23b15a 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 { 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..5906b6f988e --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/assembler/classic/FailedDeploymentIdCleanupTest.java @@ -0,0 +1,120 @@ +/** + * 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.inject.Inject; +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.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 + try { + assembler.createApplication(config.configureApplication(failingModule())); + fail("the deployment was expected to fail while starting CDI"); + } catch (final Exception expected) { + // that is the point of the test + } + + 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)); + } + + private EjbModule failingModule() { + // the unsatisfied injection point makes OWB fail the application start + return module("failing-app", BrokenSingleton.class); + } + + private EjbModule workingModule() { + return module("working-app", WorkingSingleton.class); + } + + private EjbModule module(final String moduleId, final Class beanClass) { + 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); + + 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 WorkingSingleton { + public String hello() { + return "hello"; + } + } +} From e4e60f8a129b4bdcc8de94441ef874931346adfa Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Tue, 28 Jul 2026 21:47:11 +0200 Subject: [PATCH 2/2] TOMEE-4655 - keep the rollback quiet for beans that never started The rollback added for TOMEE-4655 runs over BeanContexts that startEjbs never deployed into their container, because a CDI bootstrap failure happens before startEjbs. EjbJarBuilder assigns the container at build time while the container data only appears in Container.deploy, so the stop/undeploy loops hit a null Data and threw a NullPointerException for every singleton and stateful bean. Those were collected into the UndeployException and logged, burying the real cause under a wall of errors on what is now the most common path through this branch. Guard the two containers that lacked the check that ManagedContainer, StatelessInstanceManager and SingletonInstanceManager.undeploy already have, and skip a bean whose container was already cleared. Also tighten the test to catch DeploymentException rather than Exception, so it keeps guarding the branch it was written for, and add a case that undeploys a bean which was never deployed. --- .../openejb/assembler/classic/Assembler.java | 12 ++- .../singleton/SingletonInstanceManager.java | 7 ++ .../core/stateful/StatefulContainer.java | 6 ++ .../FailedDeploymentIdCleanupTest.java | 84 +++++++++++++++++-- 4 files changed, 101 insertions(+), 8 deletions(-) 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 6967f23b15a..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 @@ -2414,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)); } @@ -2425,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 index 5906b6f988e..8e072c388bc 100644 --- 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 @@ -17,13 +17,19 @@ 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; @@ -58,11 +64,13 @@ public void deploymentIdIsReusableAfterAFailedCdiDeployment() throws Exception { 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 Exception expected) { - // that is the point of the test + } 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); @@ -76,16 +84,55 @@ public void deploymentIdIsReusableAfterAFailedCdiDeployment() throws Exception { 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 - return module("failing-app", BrokenSingleton.class); + // 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) { + private EjbModule module(final String moduleId, final Class beanClass, final Class... others) { final EjbJar ejbJar = new EjbJar(moduleId); ejbJar.addEnterpriseBean(new SingletonBean(beanClass)); @@ -95,6 +142,19 @@ private EjbModule module(final String moduleId, final Class beanClass) { 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()); @@ -111,6 +171,20 @@ public static class BrokenSingleton { 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() {