Error transforming FacesServlet #979
Comments
|
Thanks for the well-formed report, first of all, this makes it easy to look at. I went through the class files and there is nothing changed in the shape of the class what should make this a straight-forward retransform case. I assume that there is something burried in the class file upgrade, though. The original file looks a bit fishy, for example, I do not know why the lcd_w instruction is used in the class initializer on a non-wide index, and javap does not show me some of the field references correctly but does so in the transformed version. My best guess is that there is some accidental sanitation happening through ASM which sets of the JVMs check for the transformed class file. Am I understanding you correctly that this is going away by itself in newer versions of the library? I am not quite sure I follow on although the retransformation error remains. It would be interesting to build the exact source that renders the problem with an up-to-date JDK javac to see if this issue does not occur with that version in such a case. |
Sorry, indeed misleading, let me retry - yes, this error does not occur with the newer version of the library. What I meant to say is that upgrading it in our tests was good enough for our immediate purpose, however I was not able to find the root cause of the BCI-related error. So, if someone uses the older version of this library, they will get this error, hence this issue 🙂
Not sure what you want to try, but let me know if I can assist with anything. |
|
Could you attach the working class files similarly to the non-working ones? |
|
Sure, there it is - FacesServlet_2_3_2_bytecode.zip |
|
Likely this is related to the increment in the class file version. There is nothing particularly different in the two versions besides the first one being compiled with Java 6 and the other one with Java 8. I assume that if you skipped your patch to 7, it would work. This besides the class file upgrade being allowed. I think something interfers with the validation check when comparing the class file. The compilation results are still odd, the wide bytecodes disappeared in the new version however. I am guessing now but it's probably the strangeness of the Eclipse Java compiler I am seeing here. I am just guessing but I think the removal of the wide byte code through ASM fails some transformation compatibility check. I will try to reconstruct a few scenarios next week, but if I am write there is little I can do (but patch the verifier in OpenJDK if I can pin it down to that, and then the question would be if this got backported anyways). |
|
The weird thing is that it was triggered by the release of the latest Jetty 9.4, or the one before (less likely). |
|
That does not make much sense to me either, there's nothing in the changesets even remotely related. I wonder how Jetty gets built, though. Maybe it's the compiler being used that causes the problem. |
However the problem is with a class that is compiled separately and which we haven't changed. I also couldn't find anything related in these commits, but then I realized that what has changed is not only the Jetty version, but the Jetty Docker image that was created to package it. It uses Given the original bytecode, the applied instrumentations and the exact JVM, I hope this is reproducible. Do you think you can pinpoint the exact BCI issue given those? |
|
That makes it much more likely, at least. If I understand it correctly now: using 15.0.1+9-18, the instrumentation fails every time? Is there some code that I can run (in Docker) to reproduce this behavior even? |
Yes. If you run our Jetty 9.4 integration test with myfaces-impl 2.2.12 it would fail. However, that's not very convenient for debugging purposes. I think that if you use 15.0.1+9-18 and our agent to instrument the Please let me know if there's anything else I can provide to assist with reproduction. |
|
Would I be seeing an exception? I am running the following trivial progam: public class Samle {
public static void main(String[] args) throws Exception {
Class.forName("javax.faces.webapp.FacesServlet", true, Sample.class.getClassLoader());
}
}with dependencies: <dependency>
<groupId>org.apache.myfaces.core</groupId>
<artifactId>myfaces-api</artifactId>
<version>2.2.12</version>
</dependency>
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
<version>4.0.1</version>
</dependency>The Java version I am running: openjdk version "15.0.1" 2020-10-20
OpenJDK Runtime Environment (build 15.0.1+9-18)
OpenJDK 64-Bit Server VM (build 15.0.1+9-18, mixed mode, sharing)But nothing obvious seems to be wrong. |
|
Did you attach our agent when running it or did you apply the instrumentation otherwise? |
|
Yes, I get this output, maybe I am doing it wrong? I just downloaded the agent from Central. I added a sleep in the method to make sure it's running through. |
|
Looks like your setup is good. Please set |
|
Full output: |
|
Well, what can I say - you use the same JDK and apply the same instrumentations to the same bytecode... One last thing to check before we decide it is rare enough to keep on digging - our test, where this is reproducible, attaches the agent remotely whereas you attach it in the command line. Maybe it is not the bytecode, but something else with regards to the transformation. If possible, please try to do a longer sleep and attach it remotely, hopefully this will reproduce the issue. If it still doesn't and the bytecode you get is the same, let's drop this. Thanks for all your efforts in trying to sort this out! |
|
I have it reproduced using a dynamic attachment and will be looking into it. |
|
So far I have validated: the class files' transformation results is identical for both the dynamic and static transformations yield the exact same result such that the only conclusion can be that the JVM detects an incompatible change in one of the class files. This is normally a modifier change or an added, removed method which I cannot detect. So far I could also validate that this has nothing to do with the ldc_w instruction (which ASM dissolves implicitly when passing things through its visitor) or with the class file version update. |
|
The bootstrap methods seem to trip off the verifier which is likely a bug in the verifier. The class file is strange enough (the field references are missing in the original class file, apparently this is a class file dialect that is legal but so uncommon that even I have never seen it), and I assume the bug is related to this. If it's not a big problem, I would let it slide. I don't think this is something to be seen a lot. |
|
Yeah, I believe it is good enough as long as it is restricted to this class. The fact is that a later version of this class did not cause the issue. |
|
I'd say it's worth looking into, especially since you can provide a reproduction. But I expect they'll say that the class file upgrade is unintended use. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Our Jetty 9.4 tests started failing recently, apparently following the latest 9.4 release.
Reproducing it locally, I got the following retransformation-related error:
After some investigation, it seemed that this error occurs in the attempt to retransform the
javax.faces.webapp.FacesServletclass that comes from the myfaces 2.2.12 jar. Upgrading to 2.3.2 seems to solve the issue, so I am not digging further into it at the moment, although the retransformation error remains.The original and transformed bytecode can be found in FacesServlet_bytecode.zip. A quick look didn't reveal anything to me.
We are applying this advice as well as these two.
I hope this provides enough info in order to find the problem.
The text was updated successfully, but these errors were encountered: