[
https://issues.apache.org/jira/browse/GROOVY-12440?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
James Daugherty updated GROOVY-12440:
-------------------------------------
Description:
*Affects:* every 5.x release tested, 5.0.0 through 5.1.3.
*Works in:* 4.0.33 (no NestMembers emitted) and 6.0.0.
*Related:* GROOVY-10687, GROOVY-11780.
h3. Description
Under {{{}@CompileStatic{}}}, a closure on the right-hand side of a property
assignment made inside another closure is recorded twice in the enclosing
class's {{NestMembers}} attribute. Two examples are shown below. One entry
names the real class; the other names a class that is never written. Every
nested closure compiled after it in the same class is then listed under the
wrong name, one higher than the class actually written.
{code:groovy}
{ -> holder.value = list.collect { it } }
holder.with { value = list.collect { it } }
{code}
Those closures name the outer class as their {{{}NestHost{}}}, but the outer
class doesn't list them, so the JVM rejects their direct calls to its private
methods:
{noformat}
java.lang.IllegalAccessError: class Repro$_callPrivate_closure2$_closure4 tried
to access private method
'int Repro.secret(java.lang.Integer)' ... (Type
Repro$_callPrivate_closure2$_closure4 (loader: 'app') is not a
nest member of type Repro (loader: 'app'): current type is not listed as a nest
member)
{noformat}
The failing closure can be in any later method of the class, and the method
containing the assignment never has to run. We hit this in a Grails 8
application (Groovy 5.1.3): a statically compiled service failed at startup
because an unrelated method assigned a property inside a {{with}} block, with a
{{collect}} closure on the right-hand side.
h3. Reproducer
Standalone, no build tool:
[jdaugherty/groovy5-issue-nested-closures|https://github.com/jdaugherty/groovy5-issue-nested-closures]
{code:bash}
./reproduce.sh 5.1.3 # fails
./reproduce.sh 6.0.0 # passes
{code}
{code:groovy}
import groovy.transform.CompileStatic
@CompileStatic
class Repro {
static class Holder {
String value
}
// Never called. Compiling it is enough to corrupt the NestMembers
attribute of Repro.
void assign(Holder holder) {
Closure assignment = { -> holder.value = ['a'].collect { it }.join(',')
}
assignment.call()
}
List<Integer> callPrivate() {
Closure<List<Integer>> outer = { -> [1, 2].collect { Integer i ->
secret(i) } }
outer.call()
}
private int secret(Integer i) {
i * 10
}
static void main(String[] args) {
println new Repro().callPrivate()
}
}
{code}
*Expected:* prints {{{}[10, 20]{}}}, and {{NestMembers}} lists exactly the
closure classes written.
*Actual (5.1.3):* the {{IllegalAccessError}} above. Classes written versus the
{{NestMembers}} attribute:
{noformat}
Classes written NestMembers of Repro
Repro$_assign_closure1 Repro$_assign_closure1
Repro$_assign_closure1$_closure3 Repro$_assign_closure1$_closure3
Repro$_assign_closure1$_closure4
<- never written
Repro$_callPrivate_closure2 Repro$_callPrivate_closure2
Repro$_callPrivate_closure2$_closure4 Repro$_callPrivate_closure2$_closure5
<- written as ...$_closure4
{noformat}
h3. Scope
Tested on 5.1.3, with the same {{callPrivate()}} method following each variant:
{code:groovy}
// Fails: whether or not the assignment is the closure's last statement.
// Each assignment adds one phantom entry; two assignments shift later names by
two.
{ -> holder.value = list.collect { it } }
holder.with { value = list.collect { it } }
holder.with { it.value = list.collect { it } }
// Passes
{ -> holder.setValue(list.collect { it }) } // explicit setter call
{ -> localVariable = list.collect { it } } // local variable
{ -> fieldOfThisClass = list.collect { it } } // field of the enclosing class
holder.value = list.collect { it } // same assignment directly in
the method
{ -> holder.value = 'x' } // no closure on the
right-hand side
{code}
The failing source compiled without {{@CompileStatic}} passes and produces a
correct {{NestMembers}} attribute. Reproduced on JDK 21.0.12 and 25.0.4.
h3. Analysis
{{AsmClassGenerator.moreNestmateEntries}} doesn't record the closure classes
that are actually generated. It predicts the names of closures nested in
closures by walking each closure's {{doCall}} body with a
{{CodeVisitorSupport}} and incrementing a counter for each
{{ClosureExpression}} it meets. The prediction holds only if the walk visits
each {{ClosureExpression}} exactly once.
For an assignment whose value is kept, static compilation rewrites the property
assignment into the following setter form:
{code:groovy}
def tmp = val; a.setX(tmp); tmp
{code}
{{StaticPropertyAccessHelper.transformToSetterCall}} builds that as
{{{}PoppingListOfExpressionsExpression(tmp, setX(tmp)){}}}, so the same
{{TemporaryVariableExpression}} appears twice in the tree.
{{TemporaryVariableExpression.visit}} evaluates the wrapped expression only
once when the visitor is an {{{}AsmClassGenerator{}}}. For any other visitor it
delegates straight to the wrapped expression. The nest-member walker therefore
counts the right-hand-side closure twice, while code generation writes it once.
GROOVY-11780 (backported to 5.0.x) doesn't cover this case.
{{moreNestmateEntries}} and {{TemporaryVariableExpression}} look unchanged on
master, so we haven't identified the change that makes 6.0.0 pass. Possible
fixes:
* Emit {{NestMembers}} from the closure classes actually generated rather than
predicted names.
* Have the walker skip {{ClosureExpression}} nodes it has already counted
(identity-based).
h3. Workaround
Compute the value outside the closure, or call the setter explicitly:
{code:groovy}
String joined = ['a'].collect { it }.join(',')
holder.with { value = joined }
holder.with { setValue(['a'].collect { it }.join(',')) }
{code}
was:
*Affects versions:* 5.0.0 – 5.1.3 (every 5.x release tested)
*Works in:* 4.0.33 (no `NestMembers` emitted), 6.0.0
*Related:* GROOVY-10687, GROOVY-11780
Under `@CompileStatic`, a closure on the right-hand side of a property
assignment made inside another closure is recorded twice in the enclosing
class's `NestMembers` attribute. Examples: `holder.value = list.collect \{ …
}`, or `value = …` inside `with \{ }`. One entry names the real class; the
other names a class that is never written. Every nested closure compiled after
it in the same class is then listed under the wrong name, one higher than the
class actually written.
Those closures name the outer class as their `NestHost`, but the outer class
doesn't list them, so the JVM rejects their direct calls to its private methods:
{code:java}
java.lang.IllegalAccessError: class Repro$_callPrivate_closure2$_closure4 tried
to access private method
'int Repro.secret(java.lang.Integer)' ... (Type
Repro$_callPrivate_closure2$_closure4 (loader: 'app') is not a
nest member of type Repro (loader: 'app'): current type is not listed as a nest
member){code}
The failing closure can be in any later method of the class, and the method
containing the assignment never has to run. We hit this in a Grails 8
application (Groovy 5.1.3): a `@CompileStatic` service failed at startup
because a closure in an unrelated method used
{code:java}
obj.with { prop = list.collect
{ … } } {code}
h3. Reproducer
Standalone, no build tool:
[https://github.com/jdaugherty/groovy5-issue-nested-closures]
{code:java}
./reproduce.sh 5.1.3 # fails
./reproduce.sh 6.0.0 # passes {code}
{code:java}
import groovy.transform.CompileStatic
@CompileStatic
class Repro {
static class Holder { String value }
// Never called. Compiling it is enough to corrupt the NestMembers
attribute of Repro.
void assign(Holder holder) {
Closure assignment = { -> holder.value = ['a'].collect { it }.join(',')
}
assignment.call()
}
List<Integer> callPrivate() {
Closure<List<Integer>> outer = { -> [1, 2].collect { Integer i ->
secret(i) } }
outer.call()
}
private int secret(Integer i) { i * 10 }
static void main(String[] args) { println new Repro().callPrivate()
}
} {code}
{*}Expected:{*}* prints `[10, 20]`, and `NestMembers` lists exactly the closure
classes written.
{*}Actual (5.1.3):{*}* the `IllegalAccessError` above. Classes written versus
the `NestMembers` attribute:
{code:java}
Classes written NestMembers of Repro
Repro$_assign_closure1 Repro$_assign_closure1
Repro$_assign_closure1$_closure3 Repro$_assign_closure1$_closure3
Repro$_assign_closure1$_closure4
<- never written
Repro$_callPrivate_closure2 Repro$_callPrivate_closure2
Repro$_callPrivate_closure2$_closure4 Repro$_callPrivate_closure2$_closure5
<- written as ...$_closure4 {code}
h3. Scope
On 5.1.3:
- {*}{{*}}Fails:{{*}}{*}
- `\{ -> holder.value = list.collect { … }
}`
- `holder.with { value = list.collect
{ … }
}`
- `holder.with { it.value = list.collect
{ … }
}`
- Whether or not the assignment is the closure's last statement.
- Each such assignment adds one phantom entry; two assignments shift later
names by two.
- {*}{{*}}Passes:{{*}}{*}
- `holder.setValue(list.collect \{ … })` inside a closure.
- Assigning to a local variable, or to a field of the enclosing class, inside
a closure.
- The same property assignment written directly in the method rather than in
a closure.
- A right-hand side with no closure.
- The failing source compiled without `@CompileStatic`, which also produces a
correct `NestMembers`.
Reproduced on JDK 21.0.12 and 25.0.4.
#
## Analysis
`AsmClassGenerator.moreNestmateEntries` doesn't record the closure classes that
are actually generated. It predicts the names of closures nested in closures by
walking each closure's `doCall` body with a `CodeVisitorSupport` and a running
counter (`name + "$_closure" + n++`). The prediction holds only if the walk
visits each `ClosureExpression` exactly once.
For an assignment whose value is kept, static compilation rewrites `a.x = val`
into `def tmp = val; a.setX(tmp); tmp`.
`StaticPropertyAccessHelper.transformToSetterCall` builds that as
`PoppingListOfExpressionsExpression(tmp, setX(tmp))`, so the same
`TemporaryVariableExpression` appears twice in the tree.
`TemporaryVariableExpression.visit` evaluates the wrapped expression only once
when the visitor is an `AsmClassGenerator`. For any other visitor it delegates
straight to the wrapped expression. The nest-member walker therefore counts the
right-hand-side closure twice, while code generation writes it once.
GROOVY-11780 (backported to 5.0.x) doesn't cover this case.
`moreNestmateEntries` and `TemporaryVariableExpression` look unchanged on
master, so we haven't identified the change that makes 6.0.0 pass. Possible
fixes:
- Emit `NestMembers` from the closure classes actually generated rather than
predicted names.
- Have the walker skip `ClosureExpression` nodes it has already counted
(identity-based).
#
## Workaround
Compute the value outside the closure, or call the setter explicitly:
{code:java}
String joined = ['a'].collect { it }.join(',')
holder.with { value = joined }
holder.with { setValue(['a'].collect
{ it }
.join(',')) } {code}
> closure on the right-hand side of a property assignment inside a closure
> corrupts NestMembers causing IllegalAccessError
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12440
> URL: https://issues.apache.org/jira/browse/GROOVY-12440
> Project: Groovy
> Issue Type: Bug
> Components: class generator, Static compilation
> Affects Versions: 5.0.0
> Reporter: James Daugherty
> Priority: Major
>
> *Affects:* every 5.x release tested, 5.0.0 through 5.1.3.
> *Works in:* 4.0.33 (no NestMembers emitted) and 6.0.0.
> *Related:* GROOVY-10687, GROOVY-11780.
> h3. Description
> Under {{{}@CompileStatic{}}}, a closure on the right-hand side of a property
> assignment made inside another closure is recorded twice in the enclosing
> class's {{NestMembers}} attribute. Two examples are shown below. One entry
> names the real class; the other names a class that is never written. Every
> nested closure compiled after it in the same class is then listed under the
> wrong name, one higher than the class actually written.
> {code:groovy}
> { -> holder.value = list.collect { it } }
> holder.with { value = list.collect { it } }
> {code}
> Those closures name the outer class as their {{{}NestHost{}}}, but the outer
> class doesn't list them, so the JVM rejects their direct calls to its private
> methods:
> {noformat}
> java.lang.IllegalAccessError: class Repro$_callPrivate_closure2$_closure4
> tried to access private method
> 'int Repro.secret(java.lang.Integer)' ... (Type
> Repro$_callPrivate_closure2$_closure4 (loader: 'app') is not a
> nest member of type Repro (loader: 'app'): current type is not listed as a
> nest member)
> {noformat}
> The failing closure can be in any later method of the class, and the method
> containing the assignment never has to run. We hit this in a Grails 8
> application (Groovy 5.1.3): a statically compiled service failed at startup
> because an unrelated method assigned a property inside a {{with}} block, with
> a {{collect}} closure on the right-hand side.
> h3. Reproducer
> Standalone, no build tool:
> [jdaugherty/groovy5-issue-nested-closures|https://github.com/jdaugherty/groovy5-issue-nested-closures]
> {code:bash}
> ./reproduce.sh 5.1.3 # fails
> ./reproduce.sh 6.0.0 # passes
> {code}
> {code:groovy}
> import groovy.transform.CompileStatic
> @CompileStatic
> class Repro {
> static class Holder {
> String value
> }
> // Never called. Compiling it is enough to corrupt the NestMembers
> attribute of Repro.
> void assign(Holder holder) {
> Closure assignment = { -> holder.value = ['a'].collect { it
> }.join(',') }
> assignment.call()
> }
> List<Integer> callPrivate() {
> Closure<List<Integer>> outer = { -> [1, 2].collect { Integer i ->
> secret(i) } }
> outer.call()
> }
> private int secret(Integer i) {
> i * 10
> }
> static void main(String[] args) {
> println new Repro().callPrivate()
> }
> }
> {code}
> *Expected:* prints {{{}[10, 20]{}}}, and {{NestMembers}} lists exactly the
> closure classes written.
> *Actual (5.1.3):* the {{IllegalAccessError}} above. Classes written versus
> the {{NestMembers}} attribute:
> {noformat}
> Classes written NestMembers of Repro
> Repro$_assign_closure1 Repro$_assign_closure1
> Repro$_assign_closure1$_closure3 Repro$_assign_closure1$_closure3
> Repro$_assign_closure1$_closure4
> <- never written
> Repro$_callPrivate_closure2 Repro$_callPrivate_closure2
> Repro$_callPrivate_closure2$_closure4
> Repro$_callPrivate_closure2$_closure5 <- written as ...$_closure4
> {noformat}
> h3. Scope
> Tested on 5.1.3, with the same {{callPrivate()}} method following each
> variant:
> {code:groovy}
> // Fails: whether or not the assignment is the closure's last statement.
> // Each assignment adds one phantom entry; two assignments shift later names
> by two.
> { -> holder.value = list.collect { it } }
> holder.with { value = list.collect { it } }
> holder.with { it.value = list.collect { it } }
> // Passes
> { -> holder.setValue(list.collect { it }) } // explicit setter call
> { -> localVariable = list.collect { it } } // local variable
> { -> fieldOfThisClass = list.collect { it } } // field of the enclosing
> class
> holder.value = list.collect { it } // same assignment directly
> in the method
> { -> holder.value = 'x' } // no closure on the
> right-hand side
> {code}
> The failing source compiled without {{@CompileStatic}} passes and produces a
> correct {{NestMembers}} attribute. Reproduced on JDK 21.0.12 and 25.0.4.
> h3. Analysis
> {{AsmClassGenerator.moreNestmateEntries}} doesn't record the closure classes
> that are actually generated. It predicts the names of closures nested in
> closures by walking each closure's {{doCall}} body with a
> {{CodeVisitorSupport}} and incrementing a counter for each
> {{ClosureExpression}} it meets. The prediction holds only if the walk visits
> each {{ClosureExpression}} exactly once.
> For an assignment whose value is kept, static compilation rewrites the
> property assignment into the following setter form:
> {code:groovy}
> def tmp = val; a.setX(tmp); tmp
> {code}
> {{StaticPropertyAccessHelper.transformToSetterCall}} builds that as
> {{{}PoppingListOfExpressionsExpression(tmp, setX(tmp)){}}}, so the same
> {{TemporaryVariableExpression}} appears twice in the tree.
> {{TemporaryVariableExpression.visit}} evaluates the wrapped expression only
> once when the visitor is an {{{}AsmClassGenerator{}}}. For any other visitor
> it delegates straight to the wrapped expression. The nest-member walker
> therefore counts the right-hand-side closure twice, while code generation
> writes it once.
> GROOVY-11780 (backported to 5.0.x) doesn't cover this case.
> {{moreNestmateEntries}} and {{TemporaryVariableExpression}} look unchanged on
> master, so we haven't identified the change that makes 6.0.0 pass. Possible
> fixes:
> * Emit {{NestMembers}} from the closure classes actually generated rather
> than predicted names.
> * Have the walker skip {{ClosureExpression}} nodes it has already counted
> (identity-based).
> h3. Workaround
> Compute the value outside the closure, or call the setter explicitly:
> {code:groovy}
> String joined = ['a'].collect { it }.join(',')
> holder.with { value = joined }
> holder.with { setValue(['a'].collect { it }.join(',')) }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
