Skip to content

TOMEE-4655 - roll back a failed deployment's registered ids#2850

Open
jungm wants to merge 1 commit into
apache:mainfrom
jungm:claude/tomee-4655-fix-6bffa1
Open

TOMEE-4655 - roll back a failed deployment's registered ids#2850
jungm wants to merge 1 commit into
apache:mainfrom
jungm:claude/tomee-4655-fix-6bffa1

Conversation

@jungm

@jungm jungm commented Jul 23, 2026

Copy link
Copy Markdown
Member

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 the ContainerSystem (via initEjbs) before it starts CDI. Its catch block treated two exception types specially:

} 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 getDuplicatesDuplicateDeploymentIdException 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

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant