jamesfredley commented on PR #15557:
URL: https://github.com/apache/grails-core/pull/15557#issuecomment-4606395474
### Controller-action parameter scope under `indy=false` - root cause +
workaround (commit `fb725b178b`)
Recording the details for the one Groovy 5 issue in this PR that, as far as
we know, has **no active JIRA ticket**. Until today it was attributed to
something else entirely - an AST / `VariableScope` problem (the action body
being re-parented into a `TryCatchStatement` by `ControllerActionTransformer`,
"dropping" method-parameter scope). That theory was a dead end: re-running
`VariableScopeVisitor` or re-scoping the generated wrapper did nothing. Today
we traced the real root cause and landed a workaround that builds and executes
correctly.
**Symptom:** under `-PgrailsIndy=false` on Groovy 5, invoking a controller
action declared with parameters throws `MissingPropertyException` at runtime -
but only inside a real application. Unit tests, `indy=true`, and isolated
scripts all pass.
**Root cause - the `OptimizingStatementWriter` slow path.** For a
parameterized action, `ControllerActionTransformer` generates a zero-arg
dispatch wrapper that binds the request params to **local variables** and
delegates via a hand-built `this.action(p1, p2)` call. Under classic
(`indy=false`) codegen, Groovy's `OptimizingStatementWriter` emits that
delegating call **twice**, behind a `__$stMC` ("standard metaclass") guard:
- a **fast path** (standard metaclass) that loads the bound locals correctly
(`aload`); and
- a **slow path** (non-standard metaclass) that re-resolves the same locals
as **dynamic property reads** - `this.getProperty("p1")`.
In a live application every controller's metaclass is non-standard (Spring
Security and plugins contribute to it), so the **slow path** runs, the bound
local falls through to `propertyMissing`, and you get
`MissingPropertyException`. An isolated class keeps the standard metaclass and
always hits the fast path - which is why it never reproduces standalone, and
why it was originally mis-diagnosed as a scope problem. The defect is in the
slow path's **argument codegen** (classic codegen resolves variables through
`CompileStack`, not the AST scope), so no amount of re-scoping helps.
`indy=true` uses a different call-site writer and is unaffected.
**Bytecode - before (the bug).** `javap` of a parameterized action compiled
under `indy=false`; the generated wrapper's delegating call is the fast/slow
pair, and the slow branch loads the action parameters with
`callGroovyObjectGetProperty` instead of the bound locals:
```
getstatic __$stMC // standard
metaclass? -> fast path
... // fast path: aload
of the bound locals (correct)
aload_0
invokeinterface CallSite.callGroovyObjectGetProperty //
this.getProperty("salary") <-- BUG (slow path)
checkcast java/lang/Integer
aload_0
invokeinterface CallSite.callGroovyObjectGetProperty //
this.getProperty("firstName") <-- BUG (slow path)
invokevirtual bindWithTypeConversion:(Ljava/lang/Integer;Ljava/lang/String;)
```
**Bytecode - after (the workaround).** `ControllerActionTransformer` tags
controllers that declare parameterized actions with
`org.codehaus.groovy.classgen.asm.OptimizingStatementWriter.ClassNodeSkip`.
That suppresses the `__$stMC` fast/slow fork entirely: the `getstatic __$stMC`
guard and **both** `callGroovyObjectGetProperty` re-resolutions are gone, and a
single, unconditional path is emitted that loads the bound locals directly
(`aload`) and passes them to `bindWithTypeConversion` - i.e. the old fast path,
now the only path. With the tag in place the affected `grails-test-examples`
integration tests pass under both `indy=false` and `indy=true`, and the
previous `boot4-disabled-integration-test-config.gradle` (which simply disabled
those tests) was removed.
**Status:** this is a **workaround, not a fix** of the underlying Groovy
defect. Unlike the other Groovy 5 items in this PR, it has **no JIRA ticket
yet** (it was misattributed until today). It should be removed once the
`OptimizingStatementWriter` slow-path codegen is fixed upstream. Full
root-cause writeup + standalone harness:
https://github.com/jamesfredley/groovy5-controller-action-param-scope-bug
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
