[
https://issues.apache.org/jira/browse/TOMEE-4652?focusedWorklogId=1032655&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1032655
]
ASF GitHub Bot logged work on TOMEE-4652:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 28/Jul/26 19:35
Start Date: 28/Jul/26 19:35
Worklog Time Spent: 10m
Work Description: jungm commented on PR #2849:
URL: https://github.com/apache/tomee/pull/2849#issuecomment-5108821054
Thanks — the ordering correction was right, and I verified it independently
before acting on it (`javap` on tomcat-catalina 11.0.23: `StandardContextValve`
has zero `fireRequest*` calls; `StandardHostValve.invoke` has
`fireRequestInitEvent` at 72, the Context pipeline at 113,
`throwable()`/`status()` at 195/298/307, and `fireRequestDestroyEvent` at 327).
Rather than only fix the javadoc, I moved the cleanup to
`OpenEJBSecurityListener.RequestCapturer` on the Host pipeline, which wraps all
of `StandardHostValve#invoke`. That covers `requestDestroyed`, the
`throwable()`/`status()` paths, and `<error-page>` servlets/JSPs, and it stops
pre-empting applications that complete their transaction in `requestDestroyed`.
The javadoc now describes the real ordering. This also removes the
`OpenEJBValve` call site entirely, so the "skipped if `listener.exit()` throws"
hole goes away with it.
Also addressed:
- **Already-rolled-back status:** `rollback()` now checks
`STATUS_ROLLEDBACK`/`STATUS_ROLLING_BACK` and logs at debug in that case,
warning otherwise.
- **Timeout reset pinning:** kept unconditional, with a comment explaining
why the tradeoff differs from `resetError`. Reading the timeout first isn't
available — Geronimo exposes no getter, only the package-private
`getTransactionTimeoutMilliseconds(long)`. And `setTransactionTimeout(0)`
stores a null value, which can't pin a webapp classloader the way a stored
exception's stack trace can.
- **Timeout never asserted:** you were right that the branch was untested.
`Committer` now reads the effective timeout of its own transaction and the test
distinguishes the leaked 120s from the 600s default. Worth noting the first
version of this assertion was a false green — the field is an absolute deadline
(`duration + currentTime`), not a duration, so comparing it to `120000` could
never match. Fixed to compare the remaining duration, and confirmed it fails
when the cleanup is removed.
On the merge precondition: I have **not** run the Jakarta Transactions TCK,
so the exclusions in apache/tomee-tck are untouched and this shouldn't merge on
my testing alone. `tomee-catalina` and `tomee-embedded` suites are green.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
Issue Time Tracking
-------------------
Worklog Id: (was: 1032655)
Time Spent: 0.5h (was: 20m)
> UserTransaction state leaks across pooled Tomcat threads between requests
> -------------------------------------------------------------------------
>
> Key: TOMEE-4652
> URL: https://issues.apache.org/jira/browse/TOMEE-4652
> Project: TomEE
> Issue Type: Bug
> Components: TomEE Core Server
> Reporter: Markus Jung
> Assignee: Markus Jung
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> When a servlet or JSP request leaves a {{UserTransaction}} in a non-clean
> state, the next request served on the same pooled Tomcat exec thread inherits
> that state. The victim request then either misses an expected
> {{IllegalStateException}} or gets an exception it does not expect. This is a
> leaker/victim pair: the same test fails in one vehicle and passes in the
> other, and which tests fail depends on which request lands on which thread.
> The Transactions 2.0 TCK web vehicles show this directly. At the full
> baseline (no exclusions), 49 tests run, 40 pass, 9 fail. All three
> signature-test vehicles pass, so the fault sits in {{UserTransaction}}
> handling, not in transaction propagation itself. The first failures in test
> order sit in the rollback area, before any {{setTransactionTimeout}} call
> runs, which rules out a timeout-related cause for those failures.
> Run alone on a fresh server, each area behaves correctly on its own: the
> rollback area passes 10 of 10, {{settransactiontimeout}} passes 4 of 4, and
> {{setrollbackonly}} passes 7 of 8 (its one failure, the last request in that
> area, is a victim of its own earlier request, not a new bug). There is no gap
> around commit-after-timeout: {{settransactiontimeout001}} sleeps 30 seconds
> before calling {{commit()}}, and when it reaches that call in isolation,
> {{commit()}} throws as required.
> Because a failing request poisons whichever request follows it on the same
> thread, excluding only the ids that fail at baseline just moves the failure
> onto different tests (a 9-id exclusion list leaves 4 different tests
> failing). All three areas are excluded whole in the harness so the default
> run stays stable and green.
> h2. Steps to reproduce / TCK reference
> Run the Jakarta Transactions 2.0 TCK web vehicles (servlet and JSP) against
> TomEE 11 without exclusions. Affected test classes and methods, currently
> excluded in {{runner-standalone/exclusions/transactions.txt}} in the
> apache/tomee-tck harness repo:
> *
> {{com/sun/ts/tests/jta/ee/usertransaction/rollback/UserRollbackClient.java}}
> — {{testUserRollback001}} through {{testUserRollback005}}, each {{_from_jsp}}
> and {{_from_servlet}}
> *
> {{com/sun/ts/tests/jta/ee/usertransaction/setrollbackonly/UserSetRollbackOnlyClient.java}}
> — {{testUserSetRollbackOnly001}} through {{testUserSetRollbackOnly004}},
> each {{_from_jsp}} and {{_from_servlet}}
> *
> {{com/sun/ts/tests/jta/ee/usertransaction/settransactiontimeout/UserSetTransactionTimeoutClient.java}}
> — {{testUserSetTransactionTimeout001}} and
> {{testUserSetTransactionTimeout002}}, each {{_from_jsp}} and {{_from_servlet}}
> To confirm the fix, remove these three areas from {{transactions.txt}} and
> rerun the full baseline; all 49 tests should pass regardless of
> thread-to-request assignment.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
