Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InvocationTargetException when compiling a class extending a class in the same file #117

Closed
muehmar opened this issue Nov 18, 2021 · 9 comments

Comments

@muehmar
Copy link

muehmar commented Nov 18, 2021

Expected behavior and actual behavior:

I got an InvocationTargetException for the compliation of a simple class which extends another class, which is in the same file:

java.lang.reflect.InvocationTargetException
org.joor.ReflectException: java.lang.reflect.InvocationTargetException
	at org.joor.Reflect.on(Reflect.java:914)
	at org.joor.Reflect.call(Reflect.java:583)
	at org.joor.Compile.lambda$compile$0(Compile.java:112)
	at org.joor.Compile$ClassFileManager.loadAndReturnMainClass(Compile.java:251)
	at org.joor.Compile.compile(Compile.java:111)
	at org.joor.Reflect.compile(Reflect.java:104)
	at org.joor.Reflect.compile(Reflect.java:79)
       ... some more
Caused by: java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.joor.Reflect.on(Reflect.java:910)
	... 90 more
Caused by: java.lang.NoClassDefFoundError: sample/AnotherSampleClass
	at java.lang.ClassLoader.defineClass1(Native Method)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:756)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:635)
	... 95 more
Caused by: java.lang.ClassNotFoundException: sample.AnotherSampleClass
	at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:418)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:351)

I want to test an annotation processor, which detects the superclass of an annotated class. The superclass must be in the same package as the annotated class, which means I cannot simply extend an existing class like Object or Exception or the like.

Steps to reproduce the problem:

Reflect.compile( "sample.SampleClass", "package sample; public class SampleClass extends AnotherSampleClass{} class AnotherSampleClass{}");

My real test is actually flaky, i.e. it passes sometimes but I got also the Exception. The example above is a minimal example but seems to fail always.

Versions:

  • jOOR: 0.9.14
  • Java: 8
@lukaseder
Copy link
Member

Thanks for your report. Interesting, the following test fails with Java 8, but not with Java 17:

@Test
public void x() {
    Class<?> c = Reflect.compile( "p.A", "package p; public class A extends B {} class B {}").type();
    System.out.println(c);
    System.out.println(c.getSuperclass());
}

With Java 17, it prints:

class p.A
class p.B

@lukaseder
Copy link
Member

Yeah, there's substantially different logic for Java 9+ in org.joor.Compile. I'm not sure, though, if there's also anything different in the JDK's JavaCompiler API as well? What do you think needs to be fixed for this to work for you?

Alternatively, can you just run those particular tests on a newer JDK?

@muehmar
Copy link
Author

muehmar commented Nov 19, 2021

Cause of the issue here #81 , I already use a workaround in the tests as the processor produces new files: I made the processor able to redirect the intermediate data format without invoking the generation of the files, so I do not really need the compiled output, I just need the processor to get invoked. (As a nice side-effect, this enables me to test the processor without the complete generation of the files).

I'm not sure whats causing this issue. When debugging my flaky test, I figured out that the order of classes matters when loadAndReturnMainClass is trying to load the main class. If the superclass is first, it will succeed, if the main class is first, it fails with the given Exception.

As I do not need any output of the compilation process, there is no need for my case to load any of the compiled classes. That could be an option for me, if there is no easy fix for the real problem but I'm not sure if you want to support such a case. Or probably thats already possible and I'm not aware of?

@muehmar
Copy link
Author

muehmar commented Nov 20, 2021

It seems that defineClass of the Classloader should have been called with the Superclass first. The Compile class loops over the entries in the classes map, thus the order depends on the class name as they are fetched from the map.

Here are two tests with Java 8:

This is an example which will always fail (that's your example from above):

@Test
  void x() {
    Class<?> c = Reflect.compile("p.A", "package p; public class A extends B {} class B {}").type();
    System.out.println(c);
    System.out.println(c.getSuperclass());
  }

and this one will always succeed:

@Test
  void y() {
    Class<?> c = Reflect.compile("p.D", "package p; public class D extends B{} class B{}").type();
    System.out.println(c);
    System.out.println(c.getSuperclass());
  }

I use randomly generated class names for my tests, thats why I got the flaky test. Now I could select in the corresponding test working class names to workaround this problem in my tests.

Hopefully it helps you to check if there is anything you could do in jOOR to fix it.

@lukaseder
Copy link
Member

Yeah, thanks for the reproducers. Can confirm, both run fine on JDK 17, the first one fails on JDK 8

@lukaseder
Copy link
Member

ClassFileManager::getJavaFileForOutput seems to receive the classes in lexical order, though I cannot see anything in the Javadoc guaranteeing this. If this is the case, then a workaround would be:

  • To store things in LinkedHashMap instead of HashMap
  • To compile things "in correct lexical order", instead: Reflect.compile("p.A", "package p; public class A extends B {} class B {}")

But that's a brittle thing to rely upon. Perhaps there's another way to "know" the correct order of classes without loading them? It would be possible with ASM, but I don't like the idea of adding that dependency...

@lukaseder
Copy link
Member

Well, the simplest solution I can think of is an O(n^2) algorithm that attempts to define all classes from a queue until the queue is empty, or until we've tried n times to empty the queue...

@lukaseder
Copy link
Member

Might be that there's an additional bug in the JDK 8 version, which seems unrelated. This test currently fails in the Java 8 distribution:

@Test
public void testClassLoadingOrder3() {
    Class<?> a = Reflect.compile("p.A", "package p; "
            + "public class A extends B {} "
            + "class B extends C {} "
            + "class C implements D {} "
            + "interface D extends E {} "
            + "interface E {}").type();

    Class<?> b = a.getSuperclass();
    Class<?> c = b.getSuperclass();
    assertEquals("p.A", a.getName());
    assertEquals("p.B", b.getName());
    assertEquals("p.C", c.getName());
    assertEquals(1, c.getInterfaces().length);
    assertEquals("p.D", c.getInterfaces()[0].getName());
    assertEquals(1, c.getInterfaces()[0].getInterfaces().length);
    assertEquals("p.E", c.getInterfaces()[0].getInterfaces()[0].getName());
}

p.B doesn't correctly extend p.C, so something is wrong in the class loading of these compiled classes. Might look into this some other time.

Your issue seems fixed now. Can you confirm this?

@muehmar
Copy link
Author

muehmar commented Nov 23, 2021

Didnt tried it out, but when I'm looking at the commit I'm sure it will! Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants