jungm opened a new pull request, #2849: URL: https://github.com/apache/tomee/pull/2849
## TOMEE-4652 A servlet or JSP that leaves a bean-managed `UserTransaction` incomplete leaks that transaction to the **next request served on the same pooled Tomcat exec thread**. The victim request then sees a bogus transaction status — either missing an expected `IllegalStateException` or getting a `NotSupportedException: Nested Transactions are not supported` on its own `begin()`. Which tests fail depends on which request lands on which thread, which is why the Transactions 2.0 TCK web vehicles (servlet + JSP) fail non-deterministically. ### Root cause Geronimo's `TransactionManagerImpl` keeps the thread-to-transaction association (and the per-thread transaction timeout) in `ThreadLocal`s that are only cleared by `commit()` / `rollback()`. EJBs are wrapped by container interceptors that restore the thread state at the end of the call; **plain servlets have no equivalent**, and `OpenEJBValve`'s request-teardown `finally` block cleaned up only the security context. Since Tomcat pools its worker threads, the association survives into the next request. ### Fix - **`TransactionCleanup`** (new) — rolls back and unassociates any transaction still active on the thread at request end, and resets the per-thread transaction timeout (which leaks the same way, since Geronimo only clears it inside `begin()`). If the rollback itself fails it falls back to `suspend()` so the association never survives the request. - Invoked from the request-teardown `finally` in **`OpenEJBValve`** (sync path) and **`OpenEJBSecurityListener.asyncExit()`** (async complete/error/timeout). - **`CoreUserTransaction.resetError(null)`** now `remove()`s the `ERROR` ThreadLocal instead of `set(null)`, so pooled threads don't keep an empty entry pinned. Separate hygiene issue, not the TCK cause. ### Testing `UserTransactionLeakTest` forces two sequential requests onto a single exec thread (`maxThreads=1`) and asserts both actually shared the thread (so it can't pass vacuously), that the second request sees `STATUS_NO_TRANSACTION`, and that it can still run a transaction of its own. Verified red/green: with the cleanup call removed the test fails with `expected:<[STATUS_NO_TRANSACTION]> but was:<[leaked status 0]>` and a follow-up `NotSupportedException: Nested Transactions are not supported` — matching the issue exactly; with the fix it passes. `tomee-catalina` and `tomee-embedded` suites are green. ### Notes for reviewers - The full Jakarta Transactions 2.0 TCK was not run here. To confirm end to end, remove the three excluded areas from `runner-standalone/exclusions/transactions.txt` in the `apache/tomee-tck` harness and rerun the 49-test baseline. - Pre-existing failures unrelated to this change exist on `main` in `StatefulBeanManagedTest`, `InterfaceTransactionTest`, and `TransactionPropagationTest` (confirmed identical on a clean checkout). 🤖 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]
