[
https://issues.apache.org/jira/browse/GROOVY-12387?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12387:
-------------------------------
Description:
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.
was:
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().
> 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)
