This is an automated email from the ASF dual-hosted git repository.

rzo1 pushed a commit to branch tomee-10.x
in repository https://gitbox.apache.org/repos/asf/tomee.git

commit f1151e98beea713522dff10570a6f94f586a24e4
Author: Markus Jung <[email protected]>
AuthorDate: Fri Jul 31 16:09:35 2026 +0200

    TOMEE-4655 - roll back a failed deployment's registered ids (#2850)
    
    (cherry picked from commit 68913b7e69cabf98e2ad18e74160b7bca655335a)
---
 .../openejb/assembler/classic/Assembler.java       |  20 ++-
 .../core/singleton/SingletonInstanceManager.java   |   7 +
 .../openejb/core/stateful/StatefulContainer.java   |   6 +
 .../classic/FailedDeploymentIdCleanupTest.java     | 194 +++++++++++++++++++++
 4 files changed, 224 insertions(+), 3 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 50be16d782..f47825263f 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 @@ public class Assembler extends AssemblerTool implements 
org.apache.openejb.spi.A
 
                 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 class Assembler extends AssemblerTool implements 
org.apache.openejb.spi.A
                 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 class Assembler extends AssemblerTool implements 
org.apache.openejb.spi.A
                 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 8f9552af1b..5f80a3e766 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 @@ public class SingletonInstanceManager {
     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<Instance> 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 3a328b37c9..8576f7973b 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 class StatefulContainer implements RpcContainer {
     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 0000000000..8e072c388b
--- /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";
+        }
+    }
+}

Reply via email to