TOMEE-4655 - roll back a failed deployment's registered ids#2850
Open
jungm wants to merge 1 commit into
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes 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 theContainerSystem(viainitEjbs) before it starts CDI. Itscatchblock treated two exception types specially: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. EveryBeanContextregistered before the CDI phase stayed inCoreContainerSystem.deployments. The next app reusing one of those ids then trippedgetDuplicates→DuplicateDeploymentIdExceptionbefore 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(commit51ab12b2, "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
FailedDeploymentIdCleanupTestdeploys 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:AssertionError: the failed deployment leaked its deployment id expected null, but was:<BeanContext(id=TheSharedDeploymentId)>assembler.classictests (RedeployTest,EjbRefTest, etc.)Also verified end-to-end against the Jakarta EE 11 Web Profile TCK (
enterprise-beans-30partition, Plume): 1138 tests, 0 failures, 0 errors, and zeroDuplicateDeploymentIdException. 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 separateapache/tomee-tckrepo and will be submitted there. TwoJsfClientEjblitejsfTestclasses 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