You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@grails.apache.org by ja...@apache.org on 2026/04/25 15:48:29 UTC
(grails-core) branch grails8-groovy6-canary updated (c2aa269e54 -> ddc7ea20c6)
This is an automated email from the ASF dual-hosted git repository.
jamesfredley pushed a change to branch grails8-groovy6-canary
in repository https://gitbox.apache.org/repos/asf/grails-core.git
from c2aa269e54 fix: Groovy 6 closure dispatch regression in ControllerActionTransformer
new 8e9cdbc50f fix: move Groovy 6 generic-getter guard from GormEntity trait to AST
new ddc7ea20c6 fix: serialise GSP compilation under Groovy 6 to dodge ListHashMap race
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../compiler/gorm/GormEntityTransformation.groovy | 24 +++++++++
.../org/grails/datastore/gorm/GormEntity.groovy | 24 ---------
.../compiler/gorm/GormEntityTransformSpec.groovy | 27 +++-------
.../grails/gsp/compiler/GroovyPageCompiler.groovy | 60 +++++++++++++++++++++-
4 files changed, 88 insertions(+), 47 deletions(-)
(grails-core) 02/02: fix: serialise GSP compilation under Groovy 6 to dodge ListHashMap race
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesfredley pushed a commit to branch grails8-groovy6-canary
in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit ddc7ea20c6f783a66a684603e298c85c3a461ce2
Author: James Fredley <ja...@users.noreply.github.com>
AuthorDate: Sat Apr 25 11:48:19 2026 -0400
fix: serialise GSP compilation under Groovy 6 to dodge ListHashMap race
Every CI job that compiled GSPs against Groovy 6.0.0-SNAPSHOT failed
with a Groovy compiler stack like:
General error during instruction selection: Index 3 out of bounds for length 3
java.lang.ArrayIndexOutOfBoundsException: Index 3 out of bounds for length 3
at org.codehaus.groovy.util.ListHashMap.toMap(ListHashMap.java:207)
at org.codehaus.groovy.util.ListHashMap.put(ListHashMap.java:146)
at java.base/java.util.Map.computeIfAbsent(Map.java:1067)
at org.codehaus.groovy.ast.NodeMetaDataHandler.getNodeMetaData(NodeMetaDataHandler.java:65)
at org.codehaus.groovy.ast.AnnotationNode.isTargetAllowed(AnnotationNode.java:168)
at org.codehaus.groovy.classgen.ExtendedVerifier.visitAnnotations(ExtendedVerifier.java:354)
at org.codehaus.groovy.classgen.ExtendedVerifier.visitConstructor(ExtendedVerifier.java:216)
...
at org.grails.web.pages.GroovyPageForkedCompiler.main(GroovyPageForkedCompiler.groovy:106)
`AnnotationNode.isTargetAllowed` was added in Groovy 6 (GROOVY-11838) to
honour the new default annotation targets and uses
`NodeMetaDataHandler.getNodeMetaData` (a `Map.computeIfAbsent` over an
internal `ListHashMap`) on shared `Annotation*` AST nodes. That cache
is touched concurrently by the Grails `GroovyPageCompiler` thread pool
(`Executors.newFixedThreadPool(availableProcessors() * 2)`) once shared
annotations like `@Inject`, `@CompileStatic`, etc. are seen by more
than one GSP compile at the same time, which is exactly the case for
test apps that pull in Spring/Grails compiled output. `ListHashMap` is
not designed for concurrent mutation, so the resize fails with an
`ArrayIndexOutOfBoundsException` and the entire GSP compile aborts.
Replace the unconditional `availableProcessors() * 2` thread pool with
a small `computeGspCompilerParallelism()` helper that:
* defaults to 1 worker on Groovy 6 (eliminates the race),
* defaults to `availableProcessors() * 2` on Groovy 5 and earlier
(preserves prior behaviour),
* honours `-Dgrails.gsp.compiler.parallelism=N` so callers can opt back
into parallel GSP compilation once Groovy 6 fixes the race (or
experimentally tune it down on Groovy 5).
Trade-off: a small wall-clock increase on Groovy 6 GSP compilation in
exchange for deterministic behaviour. The control knob is a single
system property, so this is easy to revert once the upstream Groovy
fix is available.
Verified locally:
./gradlew :grails-gsp-core:compileGroovy --rerun-tasks -> BUILD SUCCESSFUL
Assisted-by: claude-code:claude-opus-4-7
---
.../grails/gsp/compiler/GroovyPageCompiler.groovy | 60 +++++++++++++++++++++-
1 file changed, 58 insertions(+), 2 deletions(-)
diff --git a/grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/GroovyPageCompiler.groovy b/grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/GroovyPageCompiler.groovy
index 9ebc8e1c14..dba23e0d65 100644
--- a/grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/GroovyPageCompiler.groovy
+++ b/grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/GroovyPageCompiler.groovy
@@ -109,11 +109,25 @@ class GroovyPageCompiler {
}
compilerConfig.setTargetDirectory(targetDir)
compilerConfig.setSourceEncoding(encoding)
- ExecutorService threadPool = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() * 2)
+ // GSP compilation parallelism is intentionally configurable via the
+ // grails.gsp.compiler.parallelism system property. The default is
+ // 1 (serial) under Groovy 6 because Groovy 6.0.0-SNAPSHOT contains
+ // a thread-safety bug in org.codehaus.groovy.util.ListHashMap that
+ // surfaces during AnnotationNode.isTargetAllowed -> NodeMetaDataHandler
+ // .getNodeMetaData -> Map.computeIfAbsent on shared annotation
+ // metadata (e.g. @Inject, @CompileStatic) when multiple GSPs are
+ // compiled concurrently. The symptom is "General error during
+ // instruction selection: Index N out of bounds for length N" with
+ // an ArrayIndexOutOfBoundsException in ListHashMap.toMap. Falling
+ // back to a single thread eliminates the race at a small cost in
+ // wall-clock time. Override with -Dgrails.gsp.compiler.parallelism=N
+ // (or 0 to use availableProcessors*2) once Groovy 6 fixes this.
+ int parallelism = computeGspCompilerParallelism()
+ ExecutorService threadPool = Executors.newFixedThreadPool(parallelism)
CompletionService completionService = new ExecutorCompletionService(threadPool)
List<Future<Map>> futures = []
try {
- Integer collationLevel = Runtime.getRuntime().availableProcessors() * 2
+ Integer collationLevel = parallelism
if (srcFiles.size() < collationLevel) {
collationLevel = 1
}
@@ -174,6 +188,48 @@ class GroovyPageCompiler {
return compileGSPRegistry
}
+ /**
+ * Resolves the worker-thread count for parallel GSP compilation.
+ *
+ * Honours -Dgrails.gsp.compiler.parallelism=N. A value of 0 (or any
+ * non-positive number) means "use availableProcessors() * 2" (the
+ * historical Grails default). When the property is unset we default
+ * to 1 on Groovy 6 (see the inline comment at the call site for why)
+ * and to availableProcessors() * 2 on Groovy 5 and earlier.
+ */
+ private static int computeGspCompilerParallelism() {
+ int cores = Runtime.getRuntime().availableProcessors()
+ int defaultParallelism = isGroovy6OrLater() ? 1 : cores * 2
+
+ String override = System.getProperty('grails.gsp.compiler.parallelism')
+ if (override == null || override.isEmpty()) {
+ return defaultParallelism
+ }
+ try {
+ int requested = Integer.parseInt(override.trim())
+ if (requested <= 0) {
+ return cores * 2
+ }
+ return requested
+ } catch (NumberFormatException ignore) {
+ return defaultParallelism
+ }
+ }
+
+ private static boolean isGroovy6OrLater() {
+ String version = groovy.lang.GroovySystem.getVersion()
+ if (version == null || version.isEmpty()) {
+ return false
+ }
+ try {
+ int dot = version.indexOf('.')
+ int major = Integer.parseInt(dot >= 0 ? version.substring(0, dot) : version)
+ return major >= 6
+ } catch (NumberFormatException ignore) {
+ return false
+ }
+ }
+
/**
* Compiles an individual GSP file
*
(grails-core) 01/02: fix: move Groovy 6 generic-getter guard from GormEntity trait to AST
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesfredley pushed a commit to branch grails8-groovy6-canary
in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit 8e9cdbc50f42fe0065147303907e28aebfcedc91
Author: James Fredley <ja...@users.noreply.github.com>
AuthorDate: Sat Apr 25 11:44:27 2026 -0400
fix: move Groovy 6 generic-getter guard from GormEntity trait to AST
CI surfaced a regression in every Hibernate5 / Functional / Mongodb test
suite that exercised connection-aware entities, all failing with:
java.lang.IllegalArgumentException: Unknown entity: java.util.LinkedHashMap
at org.hibernate.internal.SessionImpl.fireDelete(...)
at AbstractHibernateGormInstanceApi.delete(...)
at GormStaticApi.delete(GormStaticApi.groovy:536)
at DataServiceConnectionRoutingSpec.deleteAllFromConnection (line 280)
That stack maps onto the cleanup helper
DataServiceRoutingProduct."secondary".list().each {
it."secondary".delete(flush: true)
}
The class-level `DataServiceRoutingProduct.secondary` was being routed
through the existing GROOVY-11829 workaround on the GormEntity trait
(`static Object get(String nameOrId)`) and correctly returned a
connection-scoped `GormStaticApi`. The instance-level `it.secondary`
however - which should resolve through the entity's
`propertyMissing(String)` to a `DelegatingGormEntityApi` - was finding
the SAME static method as its instance generic-getter under Groovy 6.
Verified directly:
metaClass.respondsTo(entity, 'get', String) ->
[public static java.lang.Object DataServiceRoutingProduct.get(java.lang.String)]
So `it.secondary` returned a `GormStaticApi` instead of a
`DelegatingGormEntityApi`. The subsequent `.delete(flush: true)` then
matched `GormStaticApi.delete(D instance)` with the `[flush: true]`
LinkedHashMap cast as `D`, which Hibernate finally rejected at
`session.delete(LinkedHashMap)`.
The same misrouting also explained the secondary failure pattern seen
across CrossLayerMultiDataSourceSpec:
java.lang.NullPointerException: Cannot invoke
"org.springframework.validation.Errors.getFieldErrors()"
because "originalErrors" is null
at HibernateRuntimeUtils.setupErrorsProperty(...:79)
`it.errors` was being similarly hijacked by the static `get(String)`
on a multi-datasource entity, leaving the `getErrors()` accessor used
by `setupErrorsProperty` returning `null` instead of a real `Errors`.
Fix
---
Drop the trait-level `static Object get(String nameOrId)` and instead
have `GormEntityTransformation` add an INSTANCE `Object get(String name)`
method directly to every `@Entity` class. Its body is a one-line
delegate to the existing `propertyMissing(String)`:
// generated on every @Entity class
public Object get(String name) { propertyMissing(name) }
Why this works:
1. Trait-merge no longer rejects the trait. We could not declare BOTH
`static get(String)` and instance `get(String)` on the trait
itself - Groovy reports "static and instance methods having the
same signature". Adding the instance overload via AST keeps it on
the entity class, where static + instance with the same name and
params is legal.
2. Instance dispatch picks the more specific candidate. Because the
instance method now lives directly on the entity class (not just
on the trait), Groovy's instance MOP finds it before falling back
to any trait-static `get(...)` method, so `it.secondary` routes
through the existing `propertyMissing` and yields the correct
`DelegatingGormEntityApi`.
3. Class-level dynamic property access still works. `Class` bean
properties (`simpleName`, `name`, `canonicalName`, ...) are
resolved by Groovy's normal Class metaclass before any
genericGetMethod is consulted, and connection-name lookups like
`Book.secondary` continue to land on the existing
`staticPropertyMissing` in GormEntity.
The trait keeps its original `static D get(Serializable id)` (the
public entity-by-id API) untouched.
Tests
-----
Updated `GormEntityTransformSpec` to assert the new shape:
- the AST-added instance `get(String)` exists and is `@Generated`,
- it is NOT static,
- the original `get(Serializable)` is still present.
The earlier tests that documented the old static-overload behaviour
(`Book.get('simpleName') == 'Book'`, etc.) were specific to the
removed shim and have been deleted alongside it.
Verified locally on Groovy 6.0.0-SNAPSHOT:
./gradlew :grails-datamapping-core:test \
:grails-data-hibernate5-core:test \
--tests 'org.grails.compiler.gorm.GormEntityTransformSpec' \
--tests 'org.apache.grails.data.testing.tck.tests.Domain*' \
--tests 'org.apache.grails.data.testing.tck.tests.CrossLayer*' \
--tests 'org.apache.grails.data.testing.tck.tests.DataService*'
-> 42 tests, 0 failures, BUILD SUCCESSFUL
Assisted-by: claude-code:claude-opus-4-7
---
.../compiler/gorm/GormEntityTransformation.groovy | 24 +++++++++++++++++++
.../org/grails/datastore/gorm/GormEntity.groovy | 24 -------------------
.../compiler/gorm/GormEntityTransformSpec.groovy | 27 +++++-----------------
3 files changed, 30 insertions(+), 45 deletions(-)
diff --git a/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/GormEntityTransformation.groovy b/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/GormEntityTransformation.groovy
index a154019d15..f121d84650 100644
--- a/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/GormEntityTransformation.groovy
+++ b/grails-datamapping-core/src/main/groovy/org/grails/compiler/gorm/GormEntityTransformation.groovy
@@ -292,6 +292,30 @@ class GormEntityTransformation extends AbstractASTTransformation implements Comp
classNode.addMethod('$static_propertyMissing', Modifier.PUBLIC | Modifier.STATIC, AstUtils.OBJECT_CLASS_NODE, propertyMissingGetParameters, null, propertyMissingGetBody)
markAsGenerated(classNode, propertyMissingNodeGetter)
+ // INSTANCE Object get(String name) - Groovy 6 GROOVY-11829 instance dispatch guard.
+ // The STATIC get(String) on the GormEntity trait is picked up by Groovy 6's instance
+ // MOP as the generic-getter for instance property access (it shows in
+ // metaClass.respondsTo(instance, 'get', String)), returning a connection-scoped
+ // GormStaticApi where the entity-level propertyMissing should have returned a
+ // DelegatingGormEntityApi. The mismatched type silently corrupts call chains like
+ // book.someConnection.delete(flush: true) - "Unknown entity: java.util.LinkedHashMap".
+ // Adding an instance overload directly to the entity class here gives instance MOP a
+ // more specific candidate than the inherited trait-static method, so it wins dispatch
+ // and delegates back to the existing instance propertyMissing. Adding via AST instead
+ // of declaring on the trait avoids the "static and instance methods having the same
+ // signature" trait-merge error since the trait still owns only the static get(String).
+ def instanceGetBody = new BlockStatement()
+ def instanceGetNameParam = new Parameter(ClassHelper.make(String), 'name')
+ def instanceGetArgs = new ArgumentListExpression(instanceGetNameParam)
+ def instanceGetMethodCall = new MethodCallExpression(new VariableExpression('this'), 'propertyMissing', instanceGetArgs)
+ instanceGetBody.addStatement(
+ new ExpressionStatement(instanceGetMethodCall)
+ )
+ def instanceGetParameters = [instanceGetNameParam] as Parameter[]
+ MethodNode instanceGetNode =
+ classNode.addMethod('get', Modifier.PUBLIC, AstUtils.OBJECT_CLASS_NODE, instanceGetParameters, null, instanceGetBody)
+ markAsGenerated(classNode, instanceGetNode)
+
// now process named query associations
// see https://grails.apache.org/docs/latest/ref/Domain%20Classes/namedQueries.html
diff --git a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormEntity.groovy b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormEntity.groovy
index 997deb6c32..6010581f82 100644
--- a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormEntity.groovy
+++ b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/GormEntity.groovy
@@ -606,30 +606,6 @@ trait GormEntity<D> implements GormValidateable, DirtyCheckable, GormEntityApi<D
currentGormStaticApi().get(id)
}
- /**
- * Groovy 6 generic-property-getter overload (see GROOVY-11829). Resolves
- * {@link Class} bean properties first, then falls back to GORM's
- * {@code propertyMissing} (handles datasource qualifiers and dynamic GORM
- * properties), and finally to {@link #get(Serializable)} for entity-by-ID
- * lookups. Throws {@link MissingPropertyException} when nothing matches so
- * Groovy's MOP can continue normal property resolution.
- */
- @Generated
- static Object get(String nameOrId) {
- MetaProperty mp = InvokerHelper.getMetaClass(Class).hasProperty(this, nameOrId)
- if (mp != null) {
- return mp.getProperty(this)
- }
- try {
- return currentGormStaticApi().propertyMissing(nameOrId)
- } catch (MissingPropertyException | IllegalStateException ignore) {
- try {
- return currentGormStaticApi().get((Serializable) nameOrId)
- } catch (IllegalStateException e) {
- throw new MissingPropertyException(nameOrId, this)
- }
- }
- }
/**
* Retrieves an object from the datastore. eg. Book.read(1)
diff --git a/grails-datamapping-core/src/test/groovy/org/grails/compiler/gorm/GormEntityTransformSpec.groovy b/grails-datamapping-core/src/test/groovy/org/grails/compiler/gorm/GormEntityTransformSpec.groovy
index 18c685edbd..45df082c9d 100644
--- a/grails-datamapping-core/src/test/groovy/org/grails/compiler/gorm/GormEntityTransformSpec.groovy
+++ b/grails-datamapping-core/src/test/groovy/org/grails/compiler/gorm/GormEntityTransformSpec.groovy
@@ -218,35 +218,20 @@ class GormEntityTransformSpec extends Specification{
}
void 'test Groovy 6 genericGetMethod regression workaround (GROOVY-11829)'() {
- expect: 'Class bean properties remain accessible via dynamic property access (the workaround target)'
+ expect: 'Class bean properties remain accessible via dynamic property access on @Entity classes under Groovy 6'
Book.simpleName == 'Book'
Book.name.endsWith('Book')
- and: 'the get(String) Groovy-6 compatibility overload exists and is @Generated'
- def getStringMethod = Book.getMethod('get', String)
- getStringMethod != null
- getStringMethod.isAnnotationPresent(Generated)
-
and: 'the original get(Serializable) overload still exists for entity-by-id lookups'
def getSerializableMethod = Book.getMethod('get', Serializable)
getSerializableMethod != null
getSerializableMethod.isAnnotationPresent(Generated)
- }
-
- void 'test get(String) throws MissingPropertyException when GORM not initialized and string is not a Class property'() {
- when: 'a name that is neither a Class property nor a known qualifier is passed'
- Book.get('definitelyNotAClassPropertyOrEntityIdABCXYZ')
-
- then: 'we do NOT leak the IllegalStateException raised by uninitialized GORM'
- thrown(MissingPropertyException)
- }
-
- void 'test get(String) returns Class bean property when name matches Class property and GORM not initialized'() {
- expect: 'explicit get("simpleName") returns the Class.simpleName because the Groovy 6 generic-getter workaround intercepts Class properties before delegating to the GORM static API'
- Book.get('simpleName') == 'Book'
- and: 'this is a documented behavior change vs Grails on Groovy 5: prior to GROOVY-11829, Book.get("simpleName") would call get(Serializable) and attempt an entity-by-id lookup. See GormEntity.get(String) docstring.'
- Book.get('canonicalName') == Book.canonicalName
+ and: 'the AST-added INSTANCE get(String) overload exists on each @Entity class to keep instance dispatch (e.g. book.someConnection) routed through propertyMissing under Groovy 6 (GROOVY-11829). Trait-level declaration would conflict with any sibling static get(String); see GormEntityTransformation.'
+ def instanceGetString = Book.getDeclaredMethod('get', String)
+ instanceGetString != null
+ !java.lang.reflect.Modifier.isStatic(instanceGetString.modifiers)
+ instanceGetString.isAnnotationPresent(Generated)
}
void 'test that all GormEntity/GormValidateable trait methods are marked as Generated'() {