Skip to content

GROOVY-12387: Indy: exception-handler combinator bypassed on ART - #2909

Merged
paulk-asert merged 1 commit into
apache:masterfrom
paulk-asert:groovy12387
Sep 9, 2026
Merged

paulk-asert merged 1 commit into
apache:masterfrom
paulk-asert:groovy12387

Conversation

@paulk-asert

Copy link
Copy Markdown
Contributor

No description provided.

@codecov-commenter

codecov-commenter commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.85714% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.1867%. Comparing base (17b99a8) to head (9802a1a).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
...java/org/codehaus/groovy/vmplugin/v8/Selector.java 63.6364% 2 Missing and 2 partials ⚠️
...g/codehaus/groovy/vmplugin/v8/IndyCatchCompat.java 91.6667% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##               master      #2909        +/-   ##
==================================================
- Coverage     71.1943%   71.1867%   -0.0076%     
- Complexity      37632      37681        +49     
==================================================
  Files            1581       1584         +3     
  Lines          136251     136385       +134     
  Branches        25311      25340        +29     
==================================================
+ Hits            97003      97088        +85     
- Misses          30511      30538        +27     
- Partials         8737       8759        +22     
Files with missing lines Coverage Δ
...g/codehaus/groovy/vmplugin/v8/IndyCatchCompat.java 91.6667% <91.6667%> (ø)
...java/org/codehaus/groovy/vmplugin/v8/Selector.java 80.6291% <63.6364%> (-0.5375%) ⬇️

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Adds an Android (ART) compatibility path for MethodHandles.catchException behavior so Indy call-site exception handling matches HotSpot/JDK semantics (GROOVY-12387), including for subclass exceptions (e.g., Groovy “no-stack” exceptions).

Changes:

  • Introduce IndyCatchCompat wrappers for GroovyObject fallback and GroovyRuntimeException unwrapping on Android.
  • Switch Selector to use the wrappers when AndroidSupport.isRunningAndroid() is true.
  • Add a dedicated test suite validating fallback + unwrapping semantics and call-site type preservation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/test/groovy/org/codehaus/groovy/vmplugin/v8/IndyCatchCompatTest.groovy Adds tests covering ART compatibility wrappers (fallback + unwrapping) and method-handle type behavior.
src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java Routes exception handling to IndyCatchCompat only on Android to avoid ART’s exact-class catch behavior.
src/main/java/org/codehaus/groovy/vmplugin/v8/IndyCatchCompat.java Implements plain-Java exception handling wrappers that are resilient to subclass exceptions on ART.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +44 to +45
* on Android; on a JVM the
* combinator stays, so this class is not loaded there.
return INVOKE_UNWRAPPING.bindTo(spread).asCollector(Object[].class, arity).asType(type);
}

static Object invokeWithFallback(final MethodHandle target, final Object receiver, final String name, final Object[] args) throws Throwable {
}
}

static Object invokeUnwrapping(final MethodHandle target, final Object[] args) throws Throwable {
Comment on lines +1172 to +1174
handle = AndroidSupport.isRunningAndroid()
? IndyCatchCompat.withGroovyObjectFallback(handle) // GROOVY-12387
: MethodHandles.catchException(handle, MissingMethodException.class, GROOVY_OBJECT_INVOKER);
@testlens-app

This comment has been minimized.

@paulk-asert
paulk-asert merged commit 8d3f678 into apache:master Sep 9, 2026
31 checks passed
@paulk-asert
paulk-asert deleted the groovy12387 branch September 9, 2026 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants