Repository navigation
Conversation
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>
📲 Install BuildsAndroid
|
Contributor
Author
|
Closing for now since the impact of the issue seems to be small |
markushi
marked this pull request as ready for review
October 22, 2026 14:29
markushi
requested review from
0xadam-brown,
adinauer,
markushi,
romtsn and
runningcode
as code owners
October 22, 2026 14:29
romtsn
reviewed
Oct 23, 2026
romtsn
approved these changes
Oct 23, 2026
romtsn
left a comment
Contributor
There was a problem hiding this comment.
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.
Member
I've added a verification task which checks the final |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
Removes the
MethodParametersattribute from the Java classes of every published module, right afterJavaCompileruns. The newStripMethodParametersaction inbuild-logicuses ASM 9.9 (the version AGP already brings) to rewrite only the class files that need it. TheJdkTargetConventionplugin attaches it to everyJavaCompiletask of modules that applycom.vanniktech.maven.publish, including the Androidcompile*JavaWithJavactasks.Only the methods that have a parameter without a name lose the attribute, because only those make D8 fail.
parameters_countmust 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
-parametersis 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
verifyMethodParameterstask inbuild-logicopens the final jar or aar of every published module, walks every class with ASM, and fails with a list ofarchive!/class methodif aMethodParametersentry has no name. An aar is opened down to its nestedclasses.jarandlibs/*.jar, so the check also covers classes that noJavaCompiletask of this build produced.checkdepends 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
MethodParametersentries 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 withNullPointerException: 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:MethodParameterssentry-8.61.1.jarsentryjar from this branchsentry-android-coreAAR from this branchsentry(782 classes) andsentry-android-corerelease (222 classes): 0 attribute blocks, 0 unnamed entries left.--rerun), with configuration cache enabled.spotlessApply apiDumpproduced no changes to the.apifiles.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-parametersgive 6 of 6 classes byte-identical, because javac then names every entry. A dry run over the compiledsentry-samples-spring-boot-jakartaclasses 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 keepsvisitParameter. Comparing sortedjavap -p -v -coutput of input and output, 5 of 781 classes differ, and only because ASM drops an emptyLocalVariableTablefrom synthetic switch-map initializers. Constant pool indices, code, frames, line numbers, signatures and annotations stay the same. For the rewritten test classes, the diff holdsMethodParameterslines only.The task catches a regression. With the strip action commented out,
:sentry:verifyMethodParametersfails 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 reports782 classes checked, no unnamed MethodParameters entry.sentry-android-corereads 222 classes out of the aar,sentry-compose34. 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:testand:sentry:test --tests "*vendor*"pass.Two traps worth knowing: ASM counts
MethodParametersas debug info, so aClassReaderpass withSKIP_DEBUGnever reports it. And theMethodParametersstring can stay in the constant pool of rewritten classes, because theClassWritercopies it from the reader. Check for the attribute withjavap -v, not grep.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
build-logictest for the round trip and for the name rule, so a future ASM upgrade cannot change bytecode unnoticed.🤖 Generated with Claude Code