This is an automated email from the ASF dual-hosted git repository. paulk-asert pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 7c8e476f3373e89891788d09aed4e7fe2e505253 Author: Kartik Kenchi <[email protected]> AuthorDate: Sun Jul 12 21:30:40 2026 +0530 GROOVY-12163: fix concurrent modification of ExpandoMetaClass mixinClasses --- src/main/java/groovy/lang/ExpandoMetaClass.java | 4 +- .../ExpandoMetaClassMixinConcurrencyTest.groovy | 90 ++++++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/main/java/groovy/lang/ExpandoMetaClass.java b/src/main/java/groovy/lang/ExpandoMetaClass.java index 99327009a2..5592b7e1fb 100644 --- a/src/main/java/groovy/lang/ExpandoMetaClass.java +++ b/src/main/java/groovy/lang/ExpandoMetaClass.java @@ -49,11 +49,11 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -298,7 +298,7 @@ public class ExpandoMetaClass extends MetaClassImpl implements GroovyObject { private final Map<String, Object> expandoSubclassMethods = new ConcurrentHashMap<>(16, 0.75f, 1); private final Map<String, MetaProperty> expandoProperties = new ConcurrentHashMap<>(16, 0.75f, 1); private ClosureStaticMetaMethod invokeStaticMethodMethod; - private final Set<MixinInMetaClass> mixinClasses = new LinkedHashSet<>(); + private final Set<MixinInMetaClass> mixinClasses = new CopyOnWriteArraySet<>(); /** * Creates an expando meta class with explicit registry behaviour and additional methods. diff --git a/src/test/groovy/groovy/lang/ExpandoMetaClassMixinConcurrencyTest.groovy b/src/test/groovy/groovy/lang/ExpandoMetaClassMixinConcurrencyTest.groovy new file mode 100644 index 0000000000..e76cddffa1 --- /dev/null +++ b/src/test/groovy/groovy/lang/ExpandoMetaClassMixinConcurrencyTest.groovy @@ -0,0 +1,90 @@ +/* + * 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 groovy.lang + +import org.codehaus.groovy.reflection.MixinInMetaClass +import org.junit.jupiter.api.Test + +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.CyclicBarrier +import java.util.concurrent.atomic.AtomicBoolean + +import static org.junit.jupiter.api.Assertions.assertTrue + +final class ExpandoMetaClassMixinConcurrencyTest { + + static class Target {} + + // Adding a mixin (addMixinClass) mutates the backing mixin set while method + // dispatch iterates it (findMixinMethod). The set must tolerate that; a plain + // LinkedHashSet threw ConcurrentModificationException from the dispatching thread. + @Test + void mixinAddIsSafeAgainstConcurrentDispatch() { + def loader = new GroovyClassLoader() + // distinct category classes so every mixin is a fresh structural add + def categories = (0..<160).collect { i -> + loader.parseClass("class Cat_${i} { def catMethod_${i}() { 'x' } }", "Cat_${i}.groovy") + } + + def emc = new ExpandoMetaClass(Target, false, true) + emc.initialize() + + // pre-grow the set so each iteration spans a wide window + (0..<40).each { MixinInMetaClass.mixinClassesToMetaClass(emc, [categories[it]]) } + + def errors = new CopyOnWriteArrayList<Throwable>() + def stop = new AtomicBoolean(false) + def emptyArgs = new Class[0] + def barrier = new CyclicBarrier(5) + + def readers = (1..4).collect { + Thread.start { + try { + barrier.await() + while (!stop.get()) { + emc.findMixinMethod('noSuchMixinMethod', emptyArgs) + } + } catch (Throwable t) { + errors << t + } finally { + stop.set(true) + } + } + } + def writer = Thread.start { + try { + barrier.await() + for (int i = 40; i < categories.size() && !stop.get(); i++) { + MixinInMetaClass.mixinClassesToMetaClass(emc, [categories[i]]) + } + } catch (Throwable t) { + errors << t + } finally { + stop.set(true) + } + } + + writer.join() + stop.set(true) + readers*.join() + + assertTrue(errors.isEmpty(), + "concurrent mixin add during dispatch iteration threw: ${errors.isEmpty() ? '' : errors.first()}") + } +}
