You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2026/08/27 23:33:44 UTC
(groovy) branch master updated: GROOVY-12308: MetaClassImpl: reject in-place additions to an initialized metaclass
This is an automated email from the ASF dual-hosted git repository.
daniellansun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new be43301a43 GROOVY-12308: MetaClassImpl: reject in-place additions to an initialized metaclass
be43301a43 is described below
commit be43301a43f6f1662157cb7424f88239ea349811
Author: Paul King <pa...@asert.com.au>
AuthorDate: Thu Aug 27 17:10:12 2026 +1000
GROOVY-12308: MetaClassImpl: reject in-place additions to an initialized metaclass
addMetaBeanProperty, addNewInstanceMethod and addNewStaticMethod mutated an
initialized MetaClassImpl in place with no version bump and no invalidation,
so call-site caches kept serving the pre-change state while fresh lookups saw
it — silently undefined behavior. Guard all three like addMetaMethod always
was, enforcing the contract MutableMetaClass already documents: additions
belong before initialize(); runtime changes belong on ExpandoMetaClass, whose
mutation window lifts the guard. EMC's lazy memoization of an inherited
expando property now runs inside that window (onSuperPropertyFoundInHierarchy
mirrors addSuperMethodIfNotOverridden, whose addMetaMethod call was always
guarded), keeping the two hierarchy-found hooks symmetric.
---
src/main/java/groovy/lang/ExpandoMetaClass.java | 5 +-
src/main/java/groovy/lang/MetaClassImpl.java | 26 +++++-
src/main/java/groovy/lang/MutableMetaClass.java | 6 +-
.../lang/MetaClassImplMutationGuardTest.groovy | 97 ++++++++++++++++++++++
4 files changed, 128 insertions(+), 6 deletions(-)
diff --git a/src/main/java/groovy/lang/ExpandoMetaClass.java b/src/main/java/groovy/lang/ExpandoMetaClass.java
index 4ff62655f6..cf5fdd6d72 100644
--- a/src/main/java/groovy/lang/ExpandoMetaClass.java
+++ b/src/main/java/groovy/lang/ExpandoMetaClass.java
@@ -434,7 +434,10 @@ public class ExpandoMetaClass extends MetaClassImpl implements GroovyObject {
/** {@inheritDoc} */
@Override
protected void onSuperPropertyFoundInHierarchy(MetaBeanProperty property) {
- addMetaBeanProperty(property);
+ // like addSuperMethodIfNotOverridden: this lazy memoization of an
+ // inherited expando property must run inside the mutation window,
+ // where the initialized guard of addMetaBeanProperty is lifted
+ performOperationOnMetaClass(() -> addMetaBeanProperty(property));
}
/** {@inheritDoc} */
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index c13ff243e2..170760a531 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -807,12 +807,18 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
/**
- * Adds an instance method to this metaclass.
+ * Adds an instance method to this metaclass. Must not be called after
+ * initialization: call-site caches are never told about the change, so
+ * the method could remain invisible to already-linked call sites.
*
* @param method The method to be added
+ * @throws RuntimeException if this metaclass is already initialized
*/
@Override
public void addNewInstanceMethod(final Method method) {
+ if (isInitialized()) {
+ throw new RuntimeException("Already initialized, cannot add new method: " + method);
+ }
CachedMethod cachedMethod = CachedMethod.find(method);
NewInstanceMetaMethod newMethod = new NewInstanceMetaMethod(cachedMethod);
addNewInstanceMethodToIndex(newMethod, metaMethodIndex.getHeader(newMethod.getDeclaringClass().getTheClass()));
@@ -826,12 +832,18 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
/**
- * Adds a static method to this metaclass.
+ * Adds a static method to this metaclass. Must not be called after
+ * initialization: call-site caches are never told about the change, so
+ * the method could remain invisible to already-linked call sites.
*
* @param method The method to be added
+ * @throws RuntimeException if this metaclass is already initialized
*/
@Override
public void addNewStaticMethod(final Method method) {
+ if (isInitialized()) {
+ throw new RuntimeException("Already initialized, cannot add new method: " + method);
+ }
CachedMethod cachedMethod = CachedMethod.find(method);
NewStaticMetaMethod newMethod = new NewStaticMetaMethod(cachedMethod);
addNewStaticMethodToIndex(newMethod, metaMethodIndex.getHeader(newMethod.getDeclaringClass().getTheClass()));
@@ -2922,12 +2934,20 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
/**
- * Adds a new MetaBeanProperty to this MetaClass
+ * Adds a new MetaBeanProperty to this MetaClass. Must not be called after
+ * initialization (mirroring {@link #addMetaMethod}): the mutation bumps no
+ * version and fires no invalidation, so call-site caches would keep
+ * serving the metaclass's pre-change state. Runtime changes belong on
+ * {@link ExpandoMetaClass}, whose mutation window lifts the restriction.
*
* @param mp The MetaBeanProperty
+ * @throws RuntimeException if this metaclass is already initialized
*/
@Override
public void addMetaBeanProperty(MetaBeanProperty mp) {
+ if (isInitialized()) {
+ throw new RuntimeException("Already initialized, cannot add new property: " + mp.getName());
+ }
MetaProperty staticProperty = establishStaticMetaProperty(mp);
if (staticProperty != null) {
staticPropertyIndex.put(mp.getName(), mp);
diff --git a/src/main/java/groovy/lang/MutableMetaClass.java b/src/main/java/groovy/lang/MutableMetaClass.java
index 304eb3f88a..8a49ce8852 100644
--- a/src/main/java/groovy/lang/MutableMetaClass.java
+++ b/src/main/java/groovy/lang/MutableMetaClass.java
@@ -61,14 +61,16 @@ public interface MutableMetaClass extends MetaClass {
void addNewStaticMethod(Method method);
/**
- * Adds a new MetaMethod to the MetaClass
+ * Adds a new MetaMethod to the MetaClass. This is only
+ * possible as long as initialise was not called.
*
* @param metaMethod The MetaMethod to add
*/
void addMetaMethod(MetaMethod metaMethod);
/**
- * Adds a new MetaBeanProperty to the MetaClass
+ * Adds a new MetaBeanProperty to the MetaClass. This is only
+ * possible as long as initialise was not called.
*
* @param metaBeanProperty The MetaBeanProperty instance
*/
diff --git a/src/test/groovy/groovy/lang/MetaClassImplMutationGuardTest.groovy b/src/test/groovy/groovy/lang/MetaClassImplMutationGuardTest.groovy
new file mode 100644
index 0000000000..14d4b190e9
--- /dev/null
+++ b/src/test/groovy/groovy/lang/MetaClassImplMutationGuardTest.groovy
@@ -0,0 +1,97 @@
+/*
+ * 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.junit.jupiter.api.Test
+
+import static org.junit.jupiter.api.Assertions.assertEquals
+import static org.junit.jupiter.api.Assertions.assertThrows
+import static org.junit.jupiter.api.Assertions.assertTrue
+
+/**
+ * GROOVY-12308: an initialized {@link MetaClassImpl} rejects in-place
+ * additions. Such mutations bump no version and fire no invalidation, so
+ * call-site caches would keep serving the pre-change state; the supported
+ * routes are build-then-initialize and {@link ExpandoMetaClass}.
+ */
+final class MetaClassImplMutationGuardTest {
+
+ static class Dummy {
+ String field = 'field'
+ }
+
+ @Test
+ void addMetaBeanPropertyRejectedAfterInitialization() {
+ def mc = new MetaClassImpl(Dummy)
+ mc.initialize()
+ def mbp = new MetaBeanProperty('extra', String, null, null)
+ def e = assertThrows(RuntimeException) { mc.addMetaBeanProperty(mbp) }
+ assertTrue(e.message.contains('Already initialized'), e.message)
+ }
+
+ @Test
+ void addNewInstanceMethodRejectedAfterInitialization() {
+ def mc = new MetaClassImpl(Dummy)
+ mc.initialize()
+ def method = DummyMethods.getDeclaredMethod('instanceExtra', Dummy)
+ def e = assertThrows(RuntimeException) { mc.addNewInstanceMethod(method) }
+ assertTrue(e.message.contains('Already initialized'), e.message)
+ }
+
+ @Test
+ void addNewStaticMethodRejectedAfterInitialization() {
+ def mc = new MetaClassImpl(Dummy)
+ mc.initialize()
+ def method = DummyMethods.getDeclaredMethod('staticExtra', Dummy)
+ def e = assertThrows(RuntimeException) { mc.addNewStaticMethod(method) }
+ assertTrue(e.message.contains('Already initialized'), e.message)
+ }
+
+ static class DummyMethods {
+ static String instanceExtra(Dummy self) { 'instanceExtra' }
+ static String staticExtra(Dummy self) { 'staticExtra' }
+ }
+
+ @Test
+ void buildThenInitializeStillWorks() {
+ def helper = new MetaClassImpl(Dummy)
+ helper.initialize()
+ def getter = helper.getMetaMethod('getField', new Object[0])
+
+ def mc = new MetaClassImpl(Dummy)
+ mc.addMetaBeanProperty(new MetaBeanProperty('alias', String, getter, null))
+ mc.initialize()
+
+ def dummy = new Dummy()
+ assertEquals('field', mc.getProperty(dummy, 'alias'))
+ }
+
+ @Test
+ void expandoMetaClassMutationUnaffected() {
+ try {
+ Dummy.metaClass.getExtra = { -> 'v1' }
+ assertEquals('v1', new Dummy().extra)
+ // second registration mutates the same EMC in place
+ Dummy.metaClass.getExtra = { -> 'v2' }
+ assertEquals('v2', new Dummy().extra)
+ } finally {
+ GroovySystem.metaClassRegistry.removeMetaClass(Dummy)
+ }
+ }
+}