Fix child/parent class enhancement issue during retransform by lujiajing1126 · Pull Request #756 · apache/skywalking-java

lujiajing1126 · 2025-05-22T01:55:02Z

Fix retransform bug if parent class has also been enhanced

Add a unit test to verify that the fix works.
Explain briefly why the bug exists and how to fix it.
If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
Update the CHANGES log.

This PR adds a unit test to reproduce a retransform issue:

When a child class extends a parent class and the both classes need to be enhanced by SkyWalking Agent, retransform will fail since getter/setter are not generated in the child class.

ReTransform class org.apache.skywalking.apm.agent.bytebuddy.biz.ChildBar failure: java.lang.UnsupportedOperationException: class redefinition failed: attempted to delete a method
// class version 52.0 (52)
-------------
// access flags 0x21

public class org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar extends org/apache/skywalking/apm/agent/bytebuddy/biz/ParentBar  implements org/apache/skywalking/apm/agent/core/plugin/interceptor/enhance/EnhancedInstance  {


  // access flags 0x1049
  public static volatile synthetic Lorg/apache/skywalking/apm/agent/bytebuddy/InstMethodsInter; $sw$_delegate$sayHelloChild1

  // access flags 0x101A
  private final static synthetic Ljava/lang/reflect/Method; cachedValue$$sw$r4cldq0$5k2gah0

  // access flags 0x42
  private volatile Ljava/lang/Object; _$EnhancedClassField_ws

  // access flags 0x1
  public <init>()V
    ALOAD 0
    INVOKESPECIAL org/apache/skywalking/apm/agent/bytebuddy/biz/ParentBar.<init> ()V
    RETURN
    MAXSTACK = 1
    MAXLOCALS = 1

  // access flags 0x1
  public sayHelloChild()Ljava/lang/String;
    GETSTATIC org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar.$sw$_delegate$sayHelloChild1 : Lorg/apache/skywalking/apm/agent/bytebuddy/InstMethodsInter;
    ALOAD 0
    ICONST_0
    ANEWARRAY java/lang/Object
    NEW org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar$$sw$auxiliary$7rsdj21
    DUP
    ALOAD 0
    INVOKESPECIAL org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar$$sw$auxiliary$7rsdj21.<init> (Lorg/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar;)V
    GETSTATIC org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar.cachedValue$$sw$r4cldq0$5k2gah0 : Ljava/lang/reflect/Method;
    INVOKEVIRTUAL org/apache/skywalking/apm/agent/bytebuddy/InstMethodsInter.intercept (Ljava/lang/Object;[Ljava/lang/Object;Ljava/util/concurrent/Callable;Ljava/lang/reflect/Method;)Ljava/lang/Object;
    CHECKCAST java/lang/String
    ARETURN
    MAXSTACK = 6
    MAXLOCALS = 1

  // access flags 0x1002
  private synthetic $sw$original$sayHelloChild$7s6edk3()Ljava/lang/String;
    LDC "Joe"
    ARETURN
    MAXSTACK = 1
    MAXLOCALS = 1

  // access flags 0x8
  static <clinit>()V
    GOTO L0
   L1
    NOP
    INVOKESTATIC java/lang/ClassLoader.getSystemClassLoader ()Ljava/lang/ClassLoader;
    LDC "net.bytebuddy.dynamic.Nexus"
    INVOKEVIRTUAL java/lang/ClassLoader.loadClass (Ljava/lang/String;)Ljava/lang/Class;
    LDC "initialize"
    ICONST_2
    ANEWARRAY java/lang/Class
    DUP
    ICONST_0
    LDC Ljava/lang/Class;.class
    AASTORE
    DUP
    ICONST_1
    GETSTATIC java/lang/Integer.TYPE : Ljava/lang/Class;
    AASTORE
    INVOKEVIRTUAL java/lang/Class.getMethod (Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Method;
    ACONST_NULL
    ICONST_2
    ANEWARRAY java/lang/Object
    DUP
    ICONST_0
    LDC Lorg/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar;.class
    AASTORE
    DUP
    ICONST_1
    LDC -905907445
    INVOKESTATIC java/lang/Integer.valueOf (I)Ljava/lang/Integer;
    AASTORE
    INVOKEVIRTUAL java/lang/reflect/Method.invoke (Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;
    POP
    LDC Lorg/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar;.class
    LDC "sayHelloChild"
    ICONST_0
    ANEWARRAY java/lang/Class
    INVOKEVIRTUAL java/lang/Class.getMethod (Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Method;
    PUTSTATIC org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar.cachedValue$$sw$r4cldq0$5k2gah0 : Ljava/lang/reflect/Method;
    RETURN
   L0
    NOP
    INVOKESTATIC java/lang/ClassLoader.getSystemClassLoader ()Ljava/lang/ClassLoader;
    LDC "net.bytebuddy.dynamic.Nexus"
    INVOKEVIRTUAL java/lang/ClassLoader.loadClass (Ljava/lang/String;)Ljava/lang/Class;
    LDC "initialize"
    ICONST_2
    ANEWARRAY java/lang/Class
    DUP
    ICONST_0
    LDC Ljava/lang/Class;.class
    AASTORE
    DUP
    ICONST_1
    GETSTATIC java/lang/Integer.TYPE : Ljava/lang/Class;
    AASTORE
    INVOKEVIRTUAL java/lang/Class.getMethod (Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Method;
    ACONST_NULL
    ICONST_2
    ANEWARRAY java/lang/Object
    DUP
    ICONST_0
    LDC Lorg/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar;.class
    AASTORE
    DUP
    ICONST_1
    LDC -805445578
    INVOKESTATIC java/lang/Integer.valueOf (I)Ljava/lang/Integer;
    AASTORE
    INVOKEVIRTUAL java/lang/reflect/Method.invoke (Ljava/lang/Object;[Ljava/lang/Object;)Ljava/lang/Object;
    POP
    GOTO L1
    MAXSTACK = 6
    MAXLOCALS = 0

  // access flags 0x1010
  final synthetic $sw$original$sayHelloChild$7s6edk3$accessor$$sw$r4cldq0()Ljava/lang/String;
    ALOAD 0
    INVOKESPECIAL org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar.$sw$original$sayHelloChild$7s6edk3 ()Ljava/lang/String;
    ARETURN
    MAXSTACK = 1
    MAXLOCALS = 1
}

Test failure: class org.apache.skywalking.apm.agent.bytebuddy.cases.ReTransform4Test.testEnhancedInstanceForMultiShotRetransform(), error: java.lang.UnsupportedOperationException: class redefinition failed: attempted to delete a method


java.lang.UnsupportedOperationException: class redefinition failed: attempted to delete a method

	at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses0(Native Method)
	at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses(InstrumentationImpl.java:169)
	at org.apache.skywalking.apm.agent.bytebuddy.cases.AbstractReTransformTest.reTransform(AbstractReTransformTest.java:43)
	at org.apache.skywalking.apm.agent.bytebuddy.cases.ReTransform4Test.testEnhancedInstanceForMultiShotRetransform(ReTransform4Test.java:52)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:231)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:55)

github-actions

license-eye has totally checked 5486 files.

Valid	Invalid	Ignored	Fixed
5240	2	244	0

Click to see the invalid file list

apm-sniffer/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar.java
apm-sniffer/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/biz/ParentBar.java

...er/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar.java

...r/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/biz/ParentBar.java

...sniffer/bytebuddy-patch/src/main/java/net/bytebuddy/agent/builder/SWDescriptionStrategy.java

lujiajing1126 · 2025-05-23T01:29:21Z

Ready to be reviewed @kylixs @wu-sheng

wu-sheng · 2025-05-23T01:37:09Z

...sniffer/bytebuddy-patch/src/main/java/net/bytebuddy/agent/builder/SWDescriptionStrategy.java

+        private final TypeDescription.Generic superGeneric;
+
+        protected ForLoadedSuperClassWrapper(TypeDescription delegation, TypeDescription.Generic superGeneric) {
+            super(null); // HACK: a trick here since type is not used anywhere in the wrapper


Has the wrapper overrided all methods of ForLoadedSuperClass? I want to guarantee that the null would not be used unexpectedly.

If yes, could you add a UT to check that? This could prevent upgrade issue if byte-buddy core adds a method in the future.

tjuwangning · 2025-05-26T03:45:51Z

An alternative solution: To avoid missing methods in subclass, use defineMethod() to explicitly add getSkyWalkingDynamicField() and setSkyWalkingDynamicField() to the class.

builder = builder.defineField(
    CONTEXT_ATTR_NAME, Object.class, ACC_PRIVATE | ACC_VOLATILE)
    .implement(EnhancedInstance.class)
    // .intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME))
    .defineMethod("getSkyWalkingDynamicField", Object.class, Visibility.PUBLIC)
    .intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME))
    .defineMethod("setSkyWalkingDynamicField", void.class, Visibility.PUBLIC).withParameters(Object.class)
    .intercept(FieldAccessor.ofField(CONTEXT_ATTR_NAME));

wu-sheng · 2025-05-26T03:48:00Z

This seems easier and not hacking? @lujiajing1126 Could you take a look at this proposal?

lujiajing1126 · 2025-05-26T06:08:48Z

This seems easier and not hacking? @lujiajing1126 Could you take a look at this proposal?

Great. It looks more straight-forward. @tjuwangning pls raise a new PR

tjuwangning · 2025-05-27T03:02:09Z

This seems easier and not hacking? @lujiajing1126 Could you take a look at this proposal?

Great. It looks more straight-forward. @tjuwangning pls raise a new PR

Here is the new PR: #757

lujiajing1126 · 2025-05-27T06:17:00Z

Closed in favor of #757

lujiajing1126 added 2 commits May 21, 2025 19:02

add test cases

695460b

fix issue by wrapping it into SWTypeDescriptionWrapper

067ef99

lujiajing1126 requested review from kylixs and wu-sheng May 22, 2025 01:55

revert byte-buddy version

cb1bd25

lujiajing1126 changed the title ~~Fix parent class enhancement issue during retransform~~ Fix child/parent class enhancement issue during retransform May 22, 2025

github-actions bot reviewed May 22, 2025

View reviewed changes

...er/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/biz/ChildBar.java Show resolved Hide resolved

...r/bytebuddy-patch/src/test/java/org/apache/skywalking/apm/agent/bytebuddy/biz/ParentBar.java Show resolved Hide resolved

fix license

fffd7a4

lujiajing1126 commented May 22, 2025

View reviewed changes

...sniffer/bytebuddy-patch/src/main/java/net/bytebuddy/agent/builder/SWDescriptionStrategy.java Outdated Show resolved Hide resolved

lujiajing1126 added bug Something isn't working core labels May 22, 2025

lujiajing1126 added this to the 9.5.0 milestone May 22, 2025

lujiajing1126 self-assigned this May 22, 2025

lujiajing1126 added 5 commits May 22, 2025 11:52

fix classloader

d241ed4

fix className

072e898

add a wrapper

69d8fb8

remove class access

8c244fd

override getDeclaredAnnotations

c99c593

wu-sheng reviewed May 23, 2025

View reviewed changes

lujiajing1126 closed this May 27, 2025

wu-sheng deleted the feature/reproduce-parent-class-enhance-issue branch May 30, 2025 12:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix child/parent class enhancement issue during retransform#756

Fix child/parent class enhancement issue during retransform#756
lujiajing1126 wants to merge 9 commits intomainfrom
feature/reproduce-parent-class-enhance-issue

lujiajing1126 commented May 22, 2025

Uh oh!

github-actions bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lujiajing1126 commented May 23, 2025

Uh oh!

wu-sheng May 23, 2025

Uh oh!

tjuwangning commented May 26, 2025

Uh oh!

wu-sheng commented May 26, 2025 •

edited

Loading

Uh oh!

lujiajing1126 commented May 26, 2025

Uh oh!

tjuwangning commented May 27, 2025

Uh oh!

lujiajing1126 commented May 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

lujiajing1126 commented May 22, 2025

Fix retransform bug if parent class has also been enhanced

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lujiajing1126 commented May 23, 2025

Uh oh!

wu-sheng May 23, 2025

Choose a reason for hiding this comment

Uh oh!

tjuwangning commented May 26, 2025

Uh oh!

wu-sheng commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lujiajing1126 commented May 26, 2025

Uh oh!

tjuwangning commented May 27, 2025

Uh oh!

lujiajing1126 commented May 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wu-sheng commented May 26, 2025 •

edited

Loading