Hi Jorren,

Thanks for the report and the patch - the analysis in the ticket is spot on, 
and the reproduction notes are appreciated.

+1 on the PR from me. synchronized (that.data) acquires the same monitor 
Collections.synchronizedMap uses internally, so the entrySet() iteration inside 
putAll is safe. 
It's minimal and back portable, and I'd like to get it into 10.x  the ticket 
has no fix version yet.

Two things I'd add to the PR before merge:

1.) ThreadContext.dataToString() has the same defect - it streams 
data.entrySet() unsynchronized, and it's reachable from 
ThreadContext.toString(), which ApplicationThreadContextSnapshot.toString() 
embeds. Worth fixing in the same commit. To keep the critical section short and 
avoid calling entry.getValue().hashCode() (arbitrary application code) while 
ho-lding a monitor that sits on the EJB invocation hot path, better to copy 
under the lock and format outside it.

2.) Separately, the copy constructor also reads that.oldClassLoader, a plain 
non-volatile field, with no happens-before. The map lock doesn't cover that, so 
the copy as a whole still isn't safely published. Not a blocker for this fix, 
just worth knowing the patch addresses the part that throws rather than the 
whole cross-thread read.

On the deeper cause: I think the CME is a symptom of 
ApplicationThreadContextSnapshot holding a live reference to another thread's 
ThreadContext instead of a captured value. currentContext() runs on the 
submitting thread but stores only the reference; the copy happens later in 
begin(), on the executor thread, while the submitter has moved on. That reads 
against the SPI: ThreadContextProvider.currentContext is specified as "Captures 
from the current thread a snapshot", returning an "immutable snapshot ... 
captured from the current thread", and ThreadContextSnapshot is "An immutable 
snapshot ... can be applied to any number of threads, including concurrently". 
Spec §4.1.1 puts the same on the provider: "Capture or produce snapshots of the 
provided typeof thread context when ThreadContextProvider methods are invoked."

Capturing eagerly in currentContext(), an immutable map plus the scalars, with 
begin() building a fresh ThreadContext per application, removes the race at the 
source and needs no locking at all. Note it can't just stash a pre-built 
ThreadContext, since ThreadContext.enter() writes into its argument and throws 
on a second entry, so one snapshot could never be applied twice.

I've build that locally and it isn't free: dropping thethreadContext != 
ThreadContext.getThreadContext() exposes an inverted restore ordering in 
ApplicationThreadContextRestorer.endContext() (it restores the TCCL, then 
ThreadContext.exit() overwrites it again), and the shallow copy makes the 
sharing of the submitter's live InvocationContext deterministic. Both are 
fixable, but it's clearly 11.x material, not a 10.x backport. I'll write it up 
on a separate ticket: https://issues.apache.org/jira/browse/TOMEE-4703

On CUTask: please do file it, and I don't think it's optional. 
initialContext.enter(), the container-listener onStart loop and 
contextService.enter(snapshot) all sit outside the try, so when enter(snapshot) 
threw the CME,taskStarting/taskAborted/taskDone never ran  which is exactly why 
you saw silent loss and not a stack trace. The finally never runs either, so 
initialContext.exit() is skipped and CUTask.CURRENT stays set on that pool 
thread. The next task scheduled onto it then hits IllegalStateException("Can't 
enter a context twice, create a new one, and call enter() on that."), so one 
CME can poison a pool thread for every task after it. That makes your second 
point a bug, independent of the map race.

Your third point (exceptions propagating into the ThreadPoolExecutor with 
nothing surfacing them) is the other half of why this was invisible. Happy to 
see that as its own ticket too; three small tickets are easier to review and 
backport than one large one.

Hope it makes sense…

Thanks again,
Richard

> Am 08.09.2026 um 16:21 schrieb Jorren Hendriks via dev <[email protected]>:
> 
> Hello everyone,
> 
> I've submitted a small patch <https://github.com/apache/tomee/pull/2939> to
> fix a concurrency issue in ThreadContext we encountered on several
> applications. The fix addresses the immediate issue and would unblock our
> other applications from migrating from TomEE 8 to 10. A review would be
> appreciated!
> 
> As mentioned in the jira ticket
> <https://issues.apache.org/jira/browse/TOMEE-4699>, this did expose a
> possibly other issue. Inside CUTask exceptions are not caught on lines
> 70-85. This means neither taskAborted or taskDone is ever called when an
> exception is thrown. Since this is not a blocking issue for me, I'll leave
> it to you to decide if it's worth changing. I can file this  as a separate
> JIRA ticket if that works better for you.
> 
> Thanks,
> Jorren
> 
> -- 
> *Confidentiality Note:* This email and any attachments may contain 
> confidential information and are intended solely for the named recipients. 
> If you are not an intended recipient, any use, copying, disclosure, or 
> distribution is prohibited. Please notify the sender and delete the message 
> and any copies. Confidential information may not be shared with any third 
> party without the prior written consent of NSX bv.

Reply via email to