Skip to content

fix(build): Strip MethodParameters so AGP 8.0 and older can dex the SDK - #6356

Open
antonis wants to merge 7 commits into
mainfrom
fix/strip-method-parameters
Open

antonis wants to merge 7 commits into
mainfrom
fix/strip-method-parameters

Conversation

@antonis

@antonis antonis commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

📜 Description

Removes the MethodParameters attribute from the Java classes of every published module, right after JavaCompile runs. The new StripMethodParameters action in build-logic uses ASM 9.9 (the version AGP already brings) to rewrite only the class files that need it. The JdkTargetConvention plugin attaches it to every JavaCompile task of modules that apply com.vanniktech.maven.publish, including the Android compile*JavaWithJavac tasks.

Only the methods that have a parameter without a name lose the attribute, because only those make D8 fail. parameters_count must match the parameters of the method descriptor, so a single entry cannot be removed; the attribute goes away for the whole method instead. Methods whose entries all have a name keep the attribute and their names, e.g. the canonical constructor of a record, or any class compiled with -parameters. Classes without an unnamed entry are not rewritten at all, and untouched methods keep the byte for byte copy ASM does. Kotlin output is unaffected.

Because of that rule, code compiled with -parameters is now safe, and the publish gate is no longer load-bearing. An earlier commit on this branch ran the action on every module while it still removed named entries too; that broke the 4 kafka-profile Spring system tests, because the sample apps compile with -parameters (Spring Boot Gradle plugin) and Spring resolves handler and bean parameters by name at start-up. The gate stays, now as defence in depth.

A task keeps it that way. The new verifyMethodParameters task in build-logic opens the final jar or aar of every published module, walks every class with ASM, and fails with a list of archive!/class method if a MethodParameters entry has no name. An aar is opened down to its nested classes.jar and libs/*.jar, so the check also covers classes that no JavaCompile task of this build produced. check depends on it, so CI runs it. Named entries pass, so the task does not forbid -parameters.

💡 Motivation and Context

Since the move to a JDK 21 toolchain in #6242, javac writes MethodParameters entries without a name for synthetic and mandated parameters, such as bridge methods (JDK-8292275). D8 older than 8.1 (AGP 8.0 and older) crashes on them with NullPointerException: Cannot invoke "String.length()", so release builds of those apps fail on 8.61.1. React Native reverted its bump in getsentry/sentry-react-native#6931 because of this.

💚 How did you test it?

Built with a JDK 21 toolchain and checked the classes with javap -v:

Artifact MethodParameters D8 3.3.75 (AGP 7.3)
Published sentry-8.61.1.jar 141 classes NPE
sentry jar from this branch 0 dexes fine
sentry-android-core AAR from this branch 0 dexes fine
  • Clean rebuild of sentry (782 classes) and sentry-android-core release (222 classes): 0 attribute blocks, 0 unnamed entries left.
  • The attribute stays stripped after a forced recompile (--rerun), with configuration cache enabled.
  • spotlessApply apiDump produced no changes to the .api files.

The name rule works in both directions. On JDK 21 test classes (enum, inner class, bridge method, record): the enum, the inner class and the bridge method lose the attribute, the record keeps its canonical constructor names x, y, and 3 of 6 classes are not rewritten at all. The same sources compiled with -parameters give 6 of 6 classes byte-identical, because javac then names every entry. A dry run over the compiled sentry-samples-spring-boot-jakarta classes rewrites 0 of 30 files.

ASM changes nothing else. The same pipeline was run over all 781 classes of sentry-8.60.0.jar, once as the stripper and once as a pure round trip that keeps visitParameter. Comparing sorted javap -p -v -c output of input and output, 5 of 781 classes differ, and only because ASM drops an empty LocalVariableTable from synthetic switch-map initializers. Constant pool indices, code, frames, line numbers, signatures and annotations stay the same. For the rewritten test classes, the diff holds MethodParameters lines only.

The task catches a regression. With the strip action commented out, :sentry:verifyMethodParameters fails and lists 199 methods, e.g. sentry-8.61.1.jar!/io/sentry/Breadcrumb.class compareTo(Ljava/lang/Object;)I. With the action back it reports 782 classes checked, no unnamed MethodParameters entry. sentry-android-core reads 222 classes out of the aar, sentry-compose 34. Sample modules do not get the task.

Published test code is safe. Published modules hold 6 Java test files (vendored gson tests and generated Apollo classes); the rest of the test sources are Kotlin. :sentry-apollo:test and :sentry:test --tests "*vendor*" pass.

Two traps worth knowing: ASM counts MethodParameters as debug info, so a ClassReader pass with SKIP_DEBUG never reports it. And the MethodParameters string can stay in the constant pool of rewritten classes, because the ClassWriter copies it from the reader. Check for the attribute with javap -v, not grep.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

  • Dex the published artifacts with an old D8 in CI, as a second line of defence.
  • Add a build-logic test for the round trip and for the name rule, so a future ASM upgrade cannot change bytecode unnoticed.
  • The system test job does not keep the start-up log of the sample apps, so a start-up failure gives no error message. Worth saving.
  • After a release with this fix, React Native can bump to it and skip 8.61.1.

🤖 Generated with Claude Code

antonis and others added 2 commits October 22, 2026 13:41
javac 21 writes nameless MethodParameters entries for synthetic and
mandated parameters (JDK-8292275), which crash D8 older than 8.1 with
a NullPointerException. Remove the attribute from the main Java classes
of published modules after compiling, restoring the javac 17 output.

Fixes #6355

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sentry

sentry Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.61.1 (1) release

⚙️ sentry-android Build Distribution Settings

@antonis

antonis commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Closing for now since the impact of the issue seems to be small

@antonis antonis closed this Oct 22, 2026
@antonis antonis reopened this Oct 22, 2026
@markushi
markushi marked this pull request as ready for review October 22, 2026 14:29
Comment thread build-logic/src/main/kotlin/io/sentry/gradle/StripMethodParameters.kt Outdated

@romtsn romtsn 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.

LGTM with one comment!

Should we also add a regression test, or is it not worth it? For example, a task that checks the published artifacts with ASM and fails the build if any class has nameless method parameters.

@getsentry getsentry deleted a comment from sentry-junior Bot Oct 23, 2026
@markushi

Copy link
Copy Markdown
Member

LGTM with one comment!

Should we also add a regression test, or is it not worth it? For example, a task that checks the published artifacts with ASM and fails the build if any class has nameless method parameters.

I've added a verification task which checks the final .jar and .aar files in the :check task.

This branch has not been deployed

No deployments
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.

8.61.1 fails to dex on AGP 8.0 and older: D8 NullPointerException "Cannot invoke String.length()"

3 participants