Avoid final field mutations in JUnit 5, Weaver and Karate instrumentations#11625
Draft
ManuelPalenzuelaDD wants to merge 1 commit into
Draft
Avoid final field mutations in JUnit 5, Weaver and Karate instrumentations#11625ManuelPalenzuelaDD wants to merge 1 commit into
ManuelPalenzuelaDD wants to merge 1 commit into
Conversation
ManuelPalenzuelaDD
added
tag: no release notes
Contributor
|
Test Environment - sbt-scalatestJob Status: success
|
Test Environment - nebula-release-pluginJob Status: success
|
Test Environment - pass4sJob Status: success
|
Test Environment - netflix-zuulJob Status: failed
|
Test Environment - sonar-kotlinJob Status: success
|
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
Test Environment - reactive-streams-jvmJob Status: failed
|
Test Environment - spring_bootJob Status: success
|
Test Environment - sonar-javaJob Status: success
|
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
1 participant
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.
What Does This Do
Removes final-field mutation from the JUnit 5, Weaver and Karate CI Visibility instrumentations and from
UnsafeUtils.tryShallowClone, so test retries / EFD / quarantine / attempt-to-fix keep working when JEP 500 flips--illegal-final-field-mutationtodeny(warns in JDK 26, denied by default later).Per mutation site:
NodeTestTask.testDescriptor/NodeTestTask.node(mutated byTestTaskHandleon retries) andAbstractTestDescriptor.uniqueId(mutated byTestDescriptorHandleto give each retry a distinct ID): thefinalmodifier is now stripped from these specific fields at class load via ByteBuddyModifierAdjustmenttype advice. The existing MethodHandle setters are unchanged — JEP 500 only forbids mutating fields that are final at runtime, and the modifier is removed before any instance of the class exists, so the JVM never relies on these fields being final.AbstractTestDescriptor.uniqueIdis covered by a newJUnit5TestDescriptorInstrumentationsince it lives in a different type than the oneJUnit5ExecutionInstrumentationmatches.SbtTask.queueis no longer overwritten after construction. The advice now intercepts the constructor and replaces the queue argument (@Advice.Argument(readOnly = false)) before it is assigned to the final field, so no field mutation happens at all. The queue is constructor parameter 5 with a stable signature across disney 0.8.4 → typelevel main; the two advices (LinkedBlockingQueue vs ConcurrentLinkedQueue) select via the argument type and only one matches per weaver version.ScenarioRuntime.result(overwritten byKarateUtils.setResultafter retries so the final result is reported): same final-stripping type advice as JUnit 5.UnsafeUtils.tryShallowClone— field copying switched fromField.setAccessible/Field.set(forbidden by JEP 500 for final fields) toUnsafefield offsets (objectFieldOffset+put*), which JEP 500 does not cover. The clone target is a fresh unpublished instance fromUnsafe.allocateInstance, so the offset writes are safe. This keeps cloning all fields (including finals) of arbitrary test descriptor classes, which the JUnit 5 retry path depends on.Motivation
JEP 500 (integrity by default) makes final-field mutation via core reflection and
MethodHandles.Lookup.unreflectSetterillegal: JDK 26 warns, and a future release flips the default todeny, at which point these instrumentations break (IllegalAccessException). Verified with the released 1.63.0 agent under--illegal-final-field-mutation=deny: agent install fails before any test runs.Part of SDTEST-3576.
Additional Notes
Unsafememory access is itself deprecated for removal (JEP 498), so theUnsafeUtilschange is a stopgap that buys time on the JEP 500 timeline; eliminatingUnsafe.allocateInstance-based cloning entirely requires a per-descriptor-class copy-constructor approach and is left for a follow-up.denybefore any test executes — independent of the tracer.Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefule labelsTesting
:internal-api:test --tests UnsafeUtilsTest✅:dd-java-agent:instrumentation:karate-1.0:test✅ (10/10, includes flaky retries / EFD / quarantine / attempt-to-fix cases):dd-java-agent:instrumentation:weaver-0.9:test,:weaver084Test,:latestDepTest✅ (29/29 — covers both queue types and latest weaver):dd-java-agent:instrumentation:junit:junit-5:junit-5.3:testfails identically on my machine on clean master (62/63 JSON-mismatch failures, pre-existing local toolchain issue) — deferring to CI for this module.