This is an automated email from the ASF dual-hosted git repository. rzo1 pushed a commit to branch tomee-10.x in repository https://gitbox.apache.org/repos/asf/tomee.git
commit 42d6fdc697689a7d2d039299ed74de2d53138f26 Author: Jorren Hendriks <[email protected]> AuthorDate: Sun Sep 20 18:08:12 2026 +0200 TOMEE-4699 Fix ConcurrentModificationException in ThreadContext constructor (#2939) * TOMEE-4699 synchronize data for safe ThreadContext copy * TOMEE-4699 Apply map synchronization to ThreadContext#dataToString * TOMEE-4699 add a test for the ThreadContext copy race Copying a ThreadContext iterates the source map. When the thread owning that context updates it at the same time the copy fails with a ConcurrentModificationException, which is what made tasks submitted to a managed executor disappear. The test copies a context on one thread while another updates it. It fails within a fraction of a second without the synchronized block in the copy constructor, and passes with it. * TOMEE-4699 Read data size and entries under the same lock --------- Co-authored-by: Richard Zowalla <[email protected]> (cherry picked from commit 183b6852be42abbf96f76f97c2f31503863475cb) --- .../org/apache/openejb/core/ThreadContext.java | 16 ++-- .../openejb/threads/ThreadContextCopyRaceTest.java | 100 +++++++++++++++++++++ 2 files changed, 111 insertions(+), 5 deletions(-) diff --git a/container/openejb-core/src/main/java/org/apache/openejb/core/ThreadContext.java b/container/openejb-core/src/main/java/org/apache/openejb/core/ThreadContext.java index 5ef55eb8a8..c421e35084 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/core/ThreadContext.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/core/ThreadContext.java @@ -154,7 +154,9 @@ public class ThreadContext { public ThreadContext(final ThreadContext that) { this.beanContext = that.beanContext; this.primaryKey = that.primaryKey; - this.data.putAll(that.data); + synchronized (that.data) { + this.data.putAll(that.data); + } this.oldClassLoader = that.oldClassLoader; } @@ -226,8 +228,7 @@ public class ThreadContext { return "ThreadContext{" + "beanContext=" + beanContext.getId() + ", primaryKey=" + primaryKey + - ", data(" + data.size() + - ")=" + dataToString(data) + + ", " + dataToString(data) + ", oldClassLoader=" + oldClassLoader + ", currentOperation=" + currentOperation + ", invokedInterface=" + invokedInterface + @@ -237,9 +238,14 @@ public class ThreadContext { } private String dataToString(final Map<Class, Object> data) { - return data.entrySet().stream() + final Map<Class, Object> copy; + // copy data under monitor (synchronized map), format outside lock + synchronized (data) { + copy = new HashMap<>(data); + } + return "data(" + copy.size() + ")=" + copy.entrySet().stream() .map(entry -> entry.getKey() + "=" + (entry.getValue() == null ? "null" : entry.getValue().hashCode())) .collect(Collectors.joining(", ")); - } + } diff --git a/container/openejb-core/src/test/java/org/apache/openejb/threads/ThreadContextCopyRaceTest.java b/container/openejb-core/src/test/java/org/apache/openejb/threads/ThreadContextCopyRaceTest.java new file mode 100644 index 0000000000..350a363747 --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/threads/ThreadContextCopyRaceTest.java @@ -0,0 +1,100 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.openejb.threads; + +import org.apache.openejb.core.ThreadContext; +import org.apache.openejb.jee.EnterpriseBean; +import org.apache.openejb.jee.SingletonBean; +import org.apache.openejb.junit.ApplicationComposer; +import org.apache.openejb.testing.Module; +import org.junit.Test; +import org.junit.runner.RunWith; + +import jakarta.ejb.EJB; +import jakarta.ejb.Singleton; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +/** + * TOMEE-4699: copying a ThreadContext iterates the source map, which fails with a + * ConcurrentModificationException when the owning thread updates it at the same time. + */ +@RunWith(ApplicationComposer.class) +public class ThreadContextCopyRaceTest { + @Module + public EnterpriseBean bean() { + return new SingletonBean(Facade.class).localBean(); + } + + @EJB + private Facade facade; + + @Test + public void copyingWhileTheOwnerUpdatesItsContext() throws Exception { + facade.hammer(); + } + + public static class Filler { + } + + @Singleton + public static class Facade { + public void hammer() throws Exception { + final ThreadContext caller = ThreadContext.getThreadContext(); + assertNotNull(caller); + + // widen the window: an entrySet iterator over a bigger map spends longer exposed + for (int i = 0; i < 16; i++) { + caller.set(Filler.class, new Filler()); + } + + final AtomicBoolean running = new AtomicBoolean(true); + final AtomicReference<Throwable> failure = new AtomicReference<>(); + + // another thread updates the context while this one copies it, which is what the + // managed executor used to do + final Thread mutator = new Thread(() -> { + try { + while (running.get()) { + caller.set(Filler.class, new Filler()); + caller.remove(Filler.class); + } + } catch (final Throwable t) { + failure.compareAndSet(null, t); + } + }, "thread-context-mutator"); + mutator.start(); + + try { + for (int i = 0; i < 200_000 && failure.get() == null; i++) { + new ThreadContext(caller); + } + } catch (final Throwable t) { + failure.compareAndSet(null, t); + } finally { + running.set(false); + mutator.join(60_000L); + } + + assertNull("copying a ThreadContext must not race with its owner: " + failure.get(), + failure.get()); + } + } +}
