[
https://issues.apache.org/jira/browse/GROOVY-12558?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18141472#comment-18141472
]
ASF GitHub Bot commented on GROOVY-12558:
-----------------------------------------
jdaugherty commented on PR #3086:
URL: https://github.com/apache/groovy/pull/3086#issuecomment-6747627782
@paulk-asert I'd like your thoughts on something I found while working out
what to document for Jochen's point above. This PR doesn't cause it, since it
only adds guards and doesn't touch method selection.
When `Y extends X` answers X's metaclass from `getMetaClass()`, the members
only Y has were reachable on 4.0, but not before or since:
| Case | 3.0.25 | 4.0.26 to 4.0.33 | 5.0.7, 5.1.2, 5.1.3, 6.0.0, this PR |
|---|---|---|---|
| `y.onlyY()` from outside | `MissingMethodException` | works |
`MissingMethodException` |
| `onlyY()` from Y's own code | `MissingMethodException` | works |
`MissingMethodException` |
| `super.method()` from Y | `StackOverflowError` | works |
`MissingMethodException` |
| Method from a trait only on Y | `MissingMethodException` | works |
`MissingMethodException` |
| Constructing a class with a stateful trait only on it |
`MissingMethodException` | works | `MissingMethodException` |
| Property only on Y | `MissingPropertyException` |
`MissingPropertyException` | `MissingPropertyException` |
These are invokedynamic results on Java 21. Classic call sites on 4.0.33,
5.1.3 and 6.0.0 give the same results, except that an implicit-`this` call in
Y's own code still works on 5.1.3 and 6.0.0, because it compiles to a direct
`invokevirtual` that skips the metaclass.
The 4.0 behaviour comes from a catch in `MetaClassImpl.invokeMethod`: on
`MissingMethodException` it looked the method up on the receiver's real class
through `GroovyObjectHelper.lookup`. That was added for JDK 16 support
(d4f1636dec, GROOVY-10145) and removed along with `$getLookup` in d465bec90e
(GROOVY-10931), first released in 5.0.0-alpha-1. The GROOVY-10931 discussion is
about illegal access and `$getLookup`, and I didn't see this receiver shape
come up, so it may have been a side effect rather than a decision. The fallback
only covered methods, which is why the property row never worked.
Generated proxies have this shape. I haven't tested a real framework, but
calling a GORM proxy's `getHibernateLazyInitializer()` from dynamic Groovy
looks like the same case.
Two questions:
1. Should the 4.0 behaviour come back, or was it an accident that 5.0
rightly ended? Either way, I'd handle any fix in its own issue, as it's in a
different code path from this PR and also affects 5.x.
2. For the documentation Jochen asked for, should we say these members
aren't found, or leave them unspecified until that's decided?
<details>
<summary>Reproducer</summary>
```groovy
class X {
String callsOwn() { 'X.callsOwn' }
String callsSuper() { 'X.callsSuper' }
}
trait Stateless { String fromTrait() { 'Stateless.fromTrait' } }
trait Stateful { String state = 'Stateful.state' }
class Y extends X implements Stateless {
@Override MetaClass getMetaClass() {
GroovySystem.metaClassRegistry.getMetaClass(X) }
String ownProp = 'Y.ownProp'
String onlyY() { 'Y.onlyY' }
@Override String callsOwn() { onlyY() }
@Override String callsSuper() { super.callsSuper() }
}
class Z extends X implements Stateful {
@Override MetaClass getMetaClass() {
GroovySystem.metaClassRegistry.getMetaClass(X) }
}
def attempt(String label, Closure c) {
try { println "$label: ${c()}" }
catch (MissingMethodException | MissingPropertyException |
StackOverflowError e) { println "$label: ${e.class.simpleName}" }
}
def y = new Y()
attempt('y.onlyY() from outside') { y.onlyY() }
attempt('onlyY() from Y') { y.callsOwn() }
attempt('super.callsSuper() from Y') { y.callsSuper() }
attempt('method from a trait only on Y') { y.fromTrait() }
attempt('new Z() with a stateful trait') { new Z(); 'constructed' }
attempt('property only on Y') { y.ownProp }
```
</details>
> Indy: call site keeps a method replaced on a metaclass the receiver shares
> with another class
> ---------------------------------------------------------------------------------------------
>
> Key: GROOVY-12558
> URL: https://issues.apache.org/jira/browse/GROOVY-12558
> Project: Groovy
> Issue Type: Bug
> Affects Versions: 6.0.0
> Reporter: James Daugherty
> Priority: Major
>
> On Groovy 6 with invokedynamic, a call site whose receiver uses the metaclass
> of another class keeps invoking a method after that metaclass replaces it.
> Groovy 5, and Groovy 6 with classic call sites, invoke the replacement. This
> is a regression from GROOVY-12191 (6.0.0-beta-1).
> Such receivers are common in tests: Spock's {{Mock()}} generates a subclass
> of the mocked type whose {{getMetaClass()}} answers the mocked type's
> metaclass, so a feature that replaces a method on the mock's metaclass
> changes the {{ExpandoMetaClass}} of the mocked type, and many proxy libraries
> behave the same way. A specification that replaces such a method in one
> feature and runs the same code under test in a later feature gets the earlier
> feature's closure: a {{0 * collaborator.update(*_)}} interaction fails with
> "Too many invocations", or an expected exception is never thrown.
> *Reproducer:*
> https://github.com/jdaugherty/groovy-indy-shared-metaclass-switchpoint-reproducer
> ({{./run.sh 5.1.3 6.0.0}}). {{GreeterProxy extends Greeter}} and is given
> the {{ExpandoMetaClass}} of {{Greeter}}, as a mock or proxy subclass is. One
> call site invokes {{greet()}} and one {{+=}} site invokes {{plus()}}; each
> method is replaced on the metaclass between two calls of the same site:
> {noformat}
> == Groovy 5.1.3
> indy=true: method call after replacing the method: second (OK)
> indy=true: compound assignment after replacing plus: second (OK)
> == Groovy 6.0.0
> indy=true: method call after replacing the method: first (STALE: the
> call site still invokes the replaced method)
> indy=true: compound assignment after replacing plus: first (STALE: the
> call site still invokes the replaced method)
> indy=false: method call after replacing the method: second (OK)
> indy=false: compound assignment after replacing plus: second (OK)
> {noformat}
> *Cause:* GROOVY-12191 replaced the global indy SwitchPoint with per-class
> domains. A call site is guarded by the SwitchPoint of the receiver's class
> ({{Selector.MethodSelector.captureTokenAndGetMetaClass()}},
> {{IndyInvalidation.classSwitchPointFor(args[0])}}; likewise in
> {{IndyCompoundAssign.resolve}}). A change to a metaclass retires the domain
> of the class the metaclass belongs to, {{MetaClass.getTheClass()}}
> ({{IndyInvalidation.switchPointForMetaClass}} / {{invalidateMetaClass}}).
> When the receiver's class is not the metaclass's class, as for
> {{GreeterProxy}} and the {{Greeter}} metaclass it uses, the change retires
> {{Greeter}}'s domain while the site waits on {{GreeterProxy}}'s. The receiver
> metaclass guard ({{SAME_MC}}) does not catch it either, because replacing an
> {{ExpandoMetaClass}} method keeps the same metaclass instance.
> *Proposed fix:* when the selected metaclass belongs to a class other than the
> receiver's, also capture that class's SwitchPoint before selection and guard
> the site with both. That applies to {{Selector}} and to
> {{IndyCompoundAssign}}, which captures its token the same way. A patch with
> tests follows as a pull request against master, with a backport for the 6.0.x
> line.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
