[
https://issues.apache.org/jira/browse/GROOVY-12558?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18136930#comment-18136930
]
ASF GitHub Bot commented on GROOVY-12558:
-----------------------------------------
jdaugherty commented on PR #3086:
URL: https://github.com/apache/groovy/pull/3086#issuecomment-6592619926
Thanks, fair questions.
GROOVY-12046 fixed a different symptom of the same receiver shape: the type
MetaClassImpl.invokeMissingMethod reports, so the indy fallback reaches
GroovyObject.invokeMethod (merged June, 6.0.0-alpha-2). GROOVY-12558 comes from
GROOVY-12191 (6.0.0-beta-1, August): a site is guarded by the SwitchPoint of
the receiver's class, while an ExpandoMetaClass edit retires only the domain of
mc.getTheClass() (ClassInfo.incVersion -> IndyInvalidation.invalidateClass).
12046 predates that.
I agree MetaClassImpl and ExpandoMetaClass were not designed to stand in for
another class. But frameworks create this shape, not test code: Spock's Mock()
answers getMetaClass() with the mocked type's registry metaclass
(JavaMockFactory/JavaMockInterceptor), GORM's Hibernate proxies return
InvokerHelper.getMetaClass(persistentClass), and a Spring CGLIB proxy of a
Groovy bean delegates getMetaClass() to its target. Groovy 5 handled these
receivers.
The fix is not ExpandoMetaClass-specific: it compares mc.getTheClass() with
the receiver's class. ExpandoMetaClass is the common trigger because it is
edited in place, so the same-metaclass guard still passes.
DelegatingMetaClass and custom metaclasses are not affected, before or after
this PR: the selector links directly only for MetaClassImpl/ExpandoMetaClass,
and any other metaclass is called through mc.invokeMethod on every call.
Spock's GroovyMock() uses such a metaclass.
Replacing greet() between two calls of one site, indy on:
| Receiver | 5.1.3 | 6.0.0 | master | with this PR |
|---|---|---|---|---|
| subclass holding the type's ExpandoMetaClass (Spock Mock()) | OK | stale |
stale | OK |
| subclass answering the type's registry metaclass (GORM proxy) |
| Spring CGLIB proxy of a Groovy bean | OK | stale | stale | OK |
| DelegatingMetaClass over the type's ExpandoMetaClass | OK | OK
| custom metaclass | OK | OK | OK | OK |
I can add the DelegatingMetaClass case and the registry-answering proxy to
the test so it covers more than one shape.
> 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)
