jungm opened a new pull request, #2850:
URL: https://github.com/apache/tomee/pull/2850

   ## What
   
   Fixes [TOMEE-4655](https://issues.apache.org/jira/browse/TOMEE-4655): a 
failed CDI/EJB deployment leaks its deployment id into later apps.
   
   ## Why
   
   `Assembler.createApplication(AppInfo, ClassLoader, boolean)` registers every 
EJB's deployment id in the `ContainerSystem` (via `initEjbs`) **before** it 
starts CDI. Its `catch` block treated two exception types specially:
   
   ```java
   } catch (final ValidationException | DeploymentException ve) {
       throw ve;                        // no cleanup
   } catch (final Throwable t) {
       destroyApplication(appInfo);     // cleanup
       ...
   }
   ```
   
   CDI startup failures bubble up as 
`jakarta.enterprise.inject.spi.DeploymentException`, so they hit the first 
clause and returned **without** undeploying the partially deployed application. 
Every `BeanContext` registered before the CDI phase stayed in 
`CoreContainerSystem.deployments`. The next app reusing one of those ids then 
tripped `getDuplicates` → `DuplicateDeploymentIdException` before any of its 
own lifecycle/managed-bean/concurrency checks ran — turning one bad deployment 
into a cascade of unrelated failures.
   
   The git history shows that clause was only ever added so these two exception 
types wouldn't be wrapped in an `OpenEJBException` (commit `51ab12b2`, *"as 
ValidationException, DeploymentException shouldn't be wrapped"*). Losing the 
rollback was an accidental side effect, not the intent.
   
   ## The fix
   
   Run `destroyApplication(appInfo)` on this path as well, then rethrow the 
original exception unchanged — preserving the "don't wrap" intent while 
releasing the ids.
   
   ## Testing
   
   New `FailedDeploymentIdCleanupTest` deploys an app whose singleton has an 
unsatisfiable `@Inject` (failing at CDI start), then asserts a second app can 
reuse the same deployment id. Verified it has real diagnostic power:
   
   - **Without the fix:** `AssertionError: the failed deployment leaked its 
deployment id expected null, but was:<BeanContext(id=TheSharedDeploymentId)>`
   - **With the fix:** passes
   - No regressions across the neighbouring `assembler.classic` tests 
(`RedeployTest`, `EjbRefTest`, etc.)
   
   Also verified end-to-end against the Jakarta EE 11 Web Profile TCK 
(`enterprise-beans-30` partition, Plume): **1138 tests, 0 failures, 0 errors**, 
and zero `DuplicateDeploymentIdException`. The four excluded test patterns the 
ticket calls out now deploy cleanly.
   
   ## Note for reviewers
   
   The matching TCK-harness changes (removing the exclusions, updating the 
expected class count, and `KNOWN_FAILURES.md`) live in the separate 
`apache/tomee-tck` repo and will be submitted there. Two 
`JsfClientEjblitejsfTest` classes among those exclusions still fail after this 
fix, but for an unrelated, already-documented reason — the OpenWebBeans "cannot 
proxy a final method" interceptor gap, which the deployment-id leak had been 
masking. They are moved into that existing exclusion block rather than left 
enabled.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to