TOMEE-4699 Fix ConcurrentModificationException in ThreadContext constructor

container/openejb-core/src/main/java/org/apache/openejb/core/ThreadContext.java

@@ -154,7 +154,9 @@ public ThreadContext(final BeanContext beanContext, final Object primaryKey, fin
     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 String toString() {
         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 String toString() {
     }
 
     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(", "));
-
     }
+
 }

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());
+        }
+    }
+}