[
https://issues.apache.org/jira/browse/GROOVY-12387?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18113220#comment-18113220
]
ASF GitHub Bot commented on GROOVY-12387:
-----------------------------------------
codecov-commenter commented on PR #2909:
URL: https://github.com/apache/groovy/pull/2909#issuecomment-5598942472
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2909?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:x: Patch coverage is `82.85714%` with `6 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 71.1914%. Comparing base
([`17b99a8`](https://app.codecov.io/gh/apache/groovy/commit/17b99a8c6c942b207257090e7ce795b34f58d8b0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`afe6359`](https://app.codecov.io/gh/apache/groovy/commit/afe63593fd268f6f78e6f13c4bf984feb0710c4e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2909?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...java/org/codehaus/groovy/vmplugin/v8/Selector.java](https://app.codecov.io/gh/apache/groovy/pull/2909?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FSelector.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L1NlbGVjdG9yLmphdmE=)
| 63.6364% | [2 Missing and 2 partials :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2909?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...g/codehaus/groovy/vmplugin/v8/IndyCatchCompat.java](https://app.codecov.io/gh/apache/groovy/pull/2909?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FIndyCatchCompat.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L0luZHlDYXRjaENvbXBhdC5qYXZh)
| 91.6667% | [2 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2909?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/2909?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2909 +/- ##
==================================================
- Coverage 71.1943% 71.1914% -0.0029%
- Complexity 37632 37633 +1
==================================================
Files 1581 1582 +1
Lines 136251 136279 +28
Branches 25311 25313 +2
==================================================
+ Hits 97003 97019 +16
- Misses 30511 30521 +10
- Partials 8737 8739 +2
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2909?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...g/codehaus/groovy/vmplugin/v8/IndyCatchCompat.java](https://app.codecov.io/gh/apache/groovy/pull/2909?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FIndyCatchCompat.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L0luZHlDYXRjaENvbXBhdC5qYXZh)
| `91.6667% <91.6667%> (ø)` | |
|
[...java/org/codehaus/groovy/vmplugin/v8/Selector.java](https://app.codecov.io/gh/apache/groovy/pull/2909?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FSelector.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L1NlbGVjdG9yLmphdmE=)
| `80.6291% <63.6364%> (-0.5375%)` | :arrow_down: |
... and [10 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2909/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> 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)
