[ 
https://issues.apache.org/jira/browse/GROOVY-12557?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18135409#comment-18135409
 ] 

ASF GitHub Bot commented on GROOVY-12557:
-----------------------------------------

paulk-asert commented on PR #3085:
URL: https://github.com/apache/groovy/pull/3085#issuecomment-6533138247

   Suggested test tweaks, fine as a follow-up after merge too:
   
   > **1. Per-instance state instead of static counters.** The counter and the 
failure switch can live on each `Watched`, which drops the `@BeforeEach` reset 
and any shared state between tests:
   > 
   > ```groovy
   > static class Watched {
   >     int metaClassCalls
   >     boolean failMetaClass
   > 
   >     @Override
   >     MetaClass getMetaClass() {
   >         metaClassCalls++
   >         if (failMetaClass) {
   >             throw new IllegalStateException('getMetaClass() called')
   >         }
   >         InvokerHelper.getMetaClass(Watched)
   >     }
   > }
   > 
   > @Test
   > void 'a cast of a value that has the target type does not call 
getMetaClass'() {
   >     Watched first = new Watched()
   >     Watched second = new Watched()
   > 
   >     assertSame first, castToWatched(first)
   >     assertSame second, castToWatched(second)
   >     assertEquals 0, first.metaClassCalls
   >     assertEquals 0, second.metaClassCalls
   > }
   > 
   > @Test
   > void 'a cast succeeds when getMetaClass of the value fails'() {
   >     Watched watched = new Watched(failMetaClass: true)
   > 
   >     assertSame watched, castToWatched(watched)
   > }
   > ```
   > 
   > **2. A proxy-shaped case.** The reported failure is a class-based proxy, 
which is a subclass of the target type. The current tests cast `Watched` to 
exactly `Watched`. Both go through the same identity handle, but one extra test 
pins the reported shape:
   > 
   > ```groovy
   > static class WatchedProxy extends Watched {
   >     @Override
   >     MetaClass getMetaClass() {
   >         throw new IllegalStateException('target out of scope')
   >     }
   > }
   > 
   > @Test
   > void 'a cast of a subclass proxy whose getMetaClass fails succeeds'() {
   >     WatchedProxy proxy = new WatchedProxy()
   > 
   >     assertSame proxy, castToWatched(proxy)
   > }
   > ```




> Indy: casting a GroovyObject calls getMetaClass() on the value being cast
> -------------------------------------------------------------------------
>
>                 Key: GROOVY-12557
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12557
>             Project: Groovy
>          Issue Type: Bug
>    Affects Versions: 4.0.33, 5.1.3, 6.0.0
>            Reporter: James Daugherty
>            Priority: Major
>
> With invokedynamic, a cast of a {{GroovyObject}} that already has the target 
> type, such as {{Widget widget = value}}, calls {{value.getMetaClass()}}: once 
> to build the call site's guard and again on each call through it. With 
> classic call sites the same cast calls nothing on the value.
> That is observable whenever {{getMetaClass()}} is not a plain field read. A 
> Spring class-based (CGLIB) proxy of a Groovy class forwards every public 
> method, {{getMetaClass()}} included, to its target. For a step-, job-, 
> request- or session-scoped bean the target exists only while the scope is 
> active, so merely assigning the proxy to a typed variable outside the scope 
> fails:
> {noformat}
> org.springframework.beans.factory.support.ScopeNotActiveException: Error 
> creating bean with name 'scopedTarget.widget': Scope 'step' is not active for 
> the current thread
>   at 
> org.springframework.aop.target.SimpleBeanTargetSource.getTarget(SimpleBeanTargetSource.java:35)
>   at 
> org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:698)
>   at Widget$$SpringCGLIB$$0.getMetaClass
>   at org.codehaus.groovy.vmplugin.v8.Selector$MethodSelector.setGuards
>   at org.codehaus.groovy.vmplugin.v8.Selector$CastSelector.castAndSetGuards
>   at org.codehaus.groovy.vmplugin.v8.Selector$CastSelector.setCallSiteTarget
>   at org.codehaus.groovy.vmplugin.v8.IndyInterface.fallback
> {noformat}
> A Grails application moving from classic call sites to invokedynamic (the 
> Grails 9 default) meets this in code that handles scoped proxies dynamically.
> *Reproducer:* 
> https://github.com/jdaugherty/groovy-indy-cast-getmetaclass-reproducer 
> ({{./run.sh 4.0.33 5.1.3 6.0.0}}). With indy the cast {{Watched typed = 
> value}} makes 2 {{getMetaClass()}} calls on the value and fails when 
> {{getMetaClass()}} throws; with classic call sites it makes none. Reproduced 
> on 4.0.33, 5.1.3 and 6.0.0.
> *Cause:* {{Selector.CastSelector.castAndSetGuards()}} reuses 
> {{MethodSelector.setGuards(args[0])}}, which calls {{getMetaClass()}} on a 
> {{GroovyObject}} receiver to bind a {{SAME_MC}} guard. Only a cast to boolean 
> selects a method ({{asBoolean}}) through the metaclass; every other cast is 
> decided by the class of the value, which the argument guards and the class 
> SwitchPoint already cover.
> *Proposed fix:* keep the receiver metaclass guard for the {{asBoolean}} path 
> only, so a cast decided by the value's class never calls into the value, as 
> with classic call sites. A patch with tests follows as a pull request against 
> master, with backports for the 6.0.x and 5.1.x lines.



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

Reply via email to