fix UnsupportedOperationException with debugger agent + retransforming agent

[SylvainJuge]

IDEA debugger agent instruments classes by modifying their structure, which is allowed before the classes are loaded for the first time.

However, when another agent tries to instrument by re-transforming one of the classes already instrumented by the debugger agent, we get UnsupportedOperationException as the modified class bytecode is not provided to the other agent.

We discovered this in the context of elastic/apm-agent-java#1673 where java.util.concurrent.ForkJoinTask#fork() is instrumented both by IDEA debugger agent and Elastic APM agent.

While we can apply a simple work-around like checking if intellij.debug.agent system property is being set and invite anyone debugging with the debugger agent active to disable it, it is definitely just a work-around and prevents us from using this very useful debugger feature. Also, that issue will happen for every class that is instrumented by this agent

Removing the if condition seems the only thing required here as:

• class transformation seems to be idempotent (not 100% sure, but I don't know how and if it's being tested).
• impact on performance is expected to be minimal as classes won't be redefined unless another agent (like ours) is active when debugging
• the agent is already declared as being able to re-transform classes (META-INF/MANIFEST.MF)

[SylvainJuge]

Also, while we haven't been able to formally test this, instrumenting the debugger agent itself with our APM agent allowed us to experiment and try to make the debugger agent instrument again, which gives us confidence that this fix allows to fix the observed issue.

[NekoCaffeine]

I mentioned this to them a long time ago, but unfortunately, they never took the issue seriously.

https://youtrack.jetbrains.com/issue/IDEA-224714

[gorrus]

Hi, thanks for your contribution!
This condition was added as a fix for https://youtrack.jetbrains.com/issue/IDEA-206160, I do not remember the details already, but obviously something will break if we simply revert it.

[NekoCaffeine]

I don’t know how to say you guys are stupid.
Have you guys actually read the documentation for the retransformer?
Because of the wrong code written by a noob, you guys broke the original normal function? This is really funny.

I don't care if this pr will eventually be merged, I've already implemented my own equivalent replacement.
But I think you guys still need to wrap up this farce.

Is there even one person in the entire JetBrains company who understands the behavior of Instrumentation?
If that's the case I suggest you guys remove this feature.

[SylvainJuge]

Thanks @gorrus for the link on the related issue.

In https://youtrack.jetbrains.com/issue/IDEA-206160, the last comment indicates that the issue might be related to using Byte Buddy, whereas in practice I don't think it's not specific to Byte Buddy but could also happen with any re-transforming agent.
I'm surprised that there hasn't had more reports like raphw/byte-buddy#594 on Byte buddy, but it might be only due to the small number of classes that are instrumented by the debugger agent.

The question here is to know if there any side-effect of applying class modification more than once with the debugger agent.
If the transformation is idempotent (and always produce the same bytecode), there should be no risk to apply it more than once, given the very small number of impacted classes we should not expect any noticeable performance impact.

Thus, if you can find more information/context on this change from ~2 years ago, that could definitely help here.

[NekoCaffeine]

@SylvainJuge I think you should look at my link(https://youtrack.jetbrains.com/issue/IDEA-224714).
Even if nothing is done, just triggering a retransform will result in an error.
I don't think we should have to suffer the consequences of their ignorance.
A JetBrains employee who doesn't understand Instrumentation and a Java noob who doesn't understand byte-buddy.
This problem has not been solved for two years, and I even plan to write an email to JetBrains to complain about the problem.
How dare you pay for such a user experience?

[klikh]

@NekoCaffeine Please refrain from using rude language and keep the conversation civil. Blaming JetBrains employees is non-constructive and won't help to resolve the issue in any way. Thank you.

[NekoCaffeine]

Ban me as JetBrains if you can. Haha.

[gorrus]

As @NekoCaffeine mentioned, the issue is well described in https://youtrack.jetbrains.com/issue/IDEA-224714.
There are some solutions proposed there:

• generate wrapper methods as private final - works only with java versions less than 13
• do not generate wrapper methods - seems to be the most correct fix (but not quick)

For now I can propose to detect "untransformed" classes on agent start, smth like:

for (Class aClass : instrumentation.getAllLoadedClasses()) {
        List<InstrumentPoint> points = myInstrumentPoints.get(Type.getInternalName(aClass));
        if (points != null) {
          for (InstrumentPoint point : points) {
            if (!point.myCapture) {
              mySkipped.add(aClass);
            }
          }
        }
      }

and then "skip" them during the transform:
if (className != null && (classBeingRedefined == null || !mySkipped.contains(classBeingRedefined))) { // we do not support redefinition or retransform
It seems to solve retransform issues until we do the "correct" fix. What do you think?

[gorrus]

implemented this in d397917, please try

[SylvainJuge]

Hi @gorrus , sorry for not giving you feedback on this sooner. I tried re-building the whole project, but I could not see if the agent version I built was working as expected here (I just got a similar behavior as the one reported in the issue), but that might definitely be due to a keyboard-and-chair issue here.

I'm not really familiar with building the whole project from scratch, is there a quick way to only recompile and package only the debugger agent ?
Also, would just replacing the agent within my current IDEA work, or is there something extra required in order to properly test it ?

[gorrus]

@SylvainJuge there's no need to build the whole project, find action "Build Artifacts" and choose debugger-agent. Then you can take the agent from out\classes\artifacts\debugger_agent and replace it in the existing installation.
Or you can try the one attached.
debugger-agent.zip