[
https://issues.apache.org/jira/browse/GROOVY-12387?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18113478#comment-18113478
]
ASF GitHub Bot commented on GROOVY-12387:
-----------------------------------------
testlens-app[bot] commented on PR #2909:
URL: https://github.com/apache/groovy/pull/2909#issuecomment-5606898134
## ✅ All tests passed ✅
🏷️ Commit: 90e84a36942c89511d91f0291f8d6dc8e7a8d5bb
▶️ Tests: 116953 executed
⚪️ Checks: 33/33 completed
---
_Learn more about TestLens at
[testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
> Indy: exception-handler combinator bypassed on ART
> --------------------------------------------------
>
> Key: GROOVY-12387
> URL: https://issues.apache.org/jira/browse/GROOVY-12387
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> Involves: the Selector wraps its GroovyObject fallback and its exception
> unwrapper in MethodHandles.catchException, which ART does not honour for
> exceptions without a writable stack trace, that is all our *ExceptionNoStack
> classes. The spike routes those three sites through a Java try/catch only
> when running on Android. A proper version would use a dedicated helper per
> handle shape rather than invokeWithArguments. The ART bug itself should also
> be reported upstream, with the repro from the app.
> Impact on normal usage: none, the JVM path is untouched behind
> AndroidSupport.isRunningAndroid().
> ART Bug details below.
> h3. Background
> On Android's ART, {{MethodHandles.catchException}} does not invoke its
> handler for an exception constructed without a writable stack trace. Verified
> on an API 36 emulator with a pure-Java probe: an exception subclass
> overriding {{fillInStackTrace()}} to return {{this}}, or constructed through
> the four-argument {{Throwable}} constructor, passes straight through the
> combinator, while an ordinary exception thrown from the same target is
> handled. Groovy's control-flow exceptions {{MissingMethodExceptionNoStack}}
> and {{MissingPropertyExceptionNoStack}} are exactly that kind, so two things
> break in the indy dispatch path on ART:
> * the GroovyObject fallback installed by
> {{Selector.setMetaClassCallHandleIfNeeded}} never runs, e.g. {{null.foo()}}
> surfaces the no-stack {{MissingMethodException}} instead of the
> {{NullPointerException}} from {{NullObject.invokeMethod}};
> * the unwrapper installed by {{Selector.MethodSelector.addExceptionHandler}}
> is skipped, so the no-stack exceptions escape unwrapped instead of becoming
> {{MissingMethodException}} / {{MissingPropertyException}} with a stack trace,
> and {{InvokerInvocationException}} causes are not unwrapped.
> Minimal repro of the ART behaviour (kept in the Android hello-world spike's
> {{ApiProbeActivity.catchExceptionChecks}}):
> {code:java}
> static class Base extends RuntimeException { ... }
> static class NoStack extends Base { @Override public Throwable
> fillInStackTrace() { return this; } }
> MethodHandle target = ...; // throws new NoStack("x")
> MethodHandle guarded = MethodHandles.catchException(target, Base.class,
> handler);
> guarded.invoke(...); // handler runs on HotSpot; on ART the NoStack propagates
> {code}
> h3. Change
> New package-private {{org.codehaus.groovy.vmplugin.v8.IndyCatchCompat}} with
> the two handlers written as plain Java {{try}}/{{catch}}:
> * {{withGroovyObjectFallback(MethodHandle)}} for the metaclass invocation
> shape {{(Object receiver, String name, Object\[\] args)Object}}: catches
> {{MissingMethodException}} and delegates to the existing
> {{IndyGuardsFiltersAndSignatures.invokeGroovyObjectInvoker}};
> * {{unwrapping(MethodHandle)}} for a call-site target of any type: spreads
> and re-collects the arguments so the target's exact type is preserved
> (primitives and {{void}} included), catches {{GroovyRuntimeException}} and
> rethrows {{ScriptBytecodeAdapter.unwrap}}'s result.
> {{Selector}} uses them at its three {{catchException}} sites only when
> {{AndroidSupport.isRunningAndroid()}} is true. On a JVM the combinator code
> is unchanged and {{IndyCatchCompat}} is never loaded, so there is no dispatch
> or native-image impact. On ART the wrappers box and collect per call, which
> is acceptable there since method handle chains are interpreted anyway.
> h3. Tests
> {{IndyCatchCompatTest}} exercises the wrappers on a JVM with the no-stack
> exceptions: the fallback runs for a matching receiver and rethrows for a
> foreign one, unrelated exceptions propagate, the unwrapper keeps the
> call-site type, converts both no-stack kinds to their stack-carrying
> counterparts, unwraps {{InvokerInvocationException}} and supports {{void}}
> targets. Existing v8 plugin, {{NullObjectTest}} and {{MethodMissingTest}}
> suites pass; checkstyle gate clean. The on-device behaviour was confirmed
> with the Android spike: with the wrappers in place the hello-world probe's
> {{null.foo()}} check reports the expected {{NullPointerException}}.
> h3. Follow-up
> Report the {{catchException}} behaviour to the Android/ART project with the
> repro above.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
