RFR(M): 8203826: Chain class initialization exceptions into later NoClassDefFoundErrors

Peter Levart peter.levart at gmail.com
Sat Jun 30 11:10:49 UTC 2018

Hi Volker,

I fully support this change, although I'm not qualified to review it as 
I'm not so acquainted with VM internals. The code looks reasonable to me 
though. This change will greatly help track down class initializiation 
bugs. I don't think that attaching a cause which is not directly an 
exception caught at the place where new exception is thrown will confuse 
people much, although I can imagine that they are used to observe such 
chains most of the time. The stack traces of cause and top exception 
clearly show where their source is and as you say the type of cause 
(ExceptionInInitializerError) hints at that too. Another hint that you 
may consider adding or not, depending on whether it would be possible to 
implement elegantly, is for the cause to somehow include the name of the 
thread in which it occurred in its message. When the top exception 
(NoClassDefFoundError) is caught and logged, the logger usually includes 
the thread name. If the cause also included thread name and it was 
different, it would be another hint to the user of what actually happened.

Just some comments / questions about the test. This is really quite a 
complicated play of exceptions, causes and initialization orders, I must 

I'll try to evaluate my reasoning by describing what test does, so let's 
start with this:

  109     public static class A {
  110         public static final int i = (new Object[1])[2].hashCode();
  111     }
  112     public static class B extends A {}
  113     public static class C extends B {}

Those 3 classes are designed so that their initialization fails. 
Initialization of A fails because ArrayIndexOutOfBoundsException is 
thrown in field initializer, initialization of B fails because it is a 
subclass of A and initialization of C fails because it is a subclass of B.

On top of that, the test includes the following class:

  115     static class ClassWithFailedInitializer {
  116         public static final int i;
  117         static {
  118             cl = new URLClassLoader(new URL[] {
}, null);
  120             try {
  121 cl.loadClass("NoClassDefFoundErrorTest$C").newInstance();
  122             } catch (ClassNotFoundException | 
InstantiationException | IllegalAccessException e) {
  123                 throw new RuntimeException("Class should be 
found", e);
  124             }
  125             i = 42;
  126         }
  127     }

...and pulls the trigger with:

  137             if (ClassWithFailedInitializer.i == 0) {

This triggers initialization of ClassWithFailedInitializer 1st, which 
constructs new ClassLoader which it then uses to load class C and 
attempts to instantiate an object from it. You then have the following 
handler arround the last two actions:

  120             try {
  121 cl.loadClass("NoClassDefFoundErrorTest$C").newInstance();
  122             } catch (ClassNotFoundException | 
InstantiationException | IllegalAccessException e) {
  123                 throw new RuntimeException("Class should be 
found", e);
  124             }

ClassNotFoundException should not be thrown as C class should be 
loadable. That's OK. The other two exceptions that you catch here are a 
little confusing, because they can only occur after successful class C 
initialization and because they are impossible exceptions in the 
concrete setup. But I can understand that you have to handle them 
somehow because they are checked exceptions. So perhaps you could group 
them into another catch block and throw RuntimeException with a more 
descriptive message (like "Impossible situation" or such). Just to be 
less confusing for someone who would have to read the code in the future...

Then we get to the following confusing point:

  136         try {
  137             if (ClassWithFailedInitializer.i == 0) {
  138                 throw new RuntimeException("Class initialization 
succeeded but is designed to fail.");
  139             }
  140             throw new Exception("Expected exception was not thrown.");
  141         }
  142         catch (ExceptionInInitializerError e) {
  143             e.printStackTrace();
  144             Asserts.assertNE(e.getCause(), null, "Expecting cause 
in ExceptionInInitializerError.");
  145             Asserts.assertEQ(e.getCause().getClass(), 
ArrayIndexOutOfBoundsException.class, "sanity");
  146         }

I think that line 138 is not reachable. If ClassWithFailedInitializer.i 
== 0 evaluated successfully, it would mean that 
ClassWithFailedInitializer initialization finished normally, but if it 
finished normally, then field i would contain 42, wouldn't it? I think 
you wanted the test to be if (ClassWithFailedInitializer.i == 42) and 
then you don't need line 140. Am I right? The same goes for line 156.

Good work.

Regards, Peter

On 06/29/18 16:53, Volker Simonis wrote:
> Hi,
> can I please have a review for the following change which saves
> ExceptionInInitializerError thrown during class initialization and
> chains them as cause into potential NoClassDefFoundErrors for the same
> class. We are using this features since years in our commercial SAP
> JVM and it proved extremely useful for detecting and fixing errors
> especially in big deployments.
> This is a follow-up on a discussion previously started by Goetz [1].
> His first proposal (which is close to our current, internal
> implementation) inserted an additional field into java.lang.Class
> objects to save potential ExceptionInInitializerErrors. This was
> considered to much overhead in the initial discussion [1].
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8203826.v2/
> https://bugs.openjdk.java.net/browse/JDK-8203826
> So in this change, I've completely re-implemented the feature by using
> a java.lang.Hashtable which is attached to the ClassLoaderData object.
> The Hashtable is lazily created when the first
> ExceptionInInitializerError is thrown and maps the Class which
> triggered the ExceptionInInitializerError during the execution of its
> static initializer to the corresponding ExceptionInInitializerError.
> If the same class will be accessed once again, this will directly lead
> to a plain NoClassDefFoundError (as per the JVMS, 5.5 Initialization)
> because the static initializer won't be executed a second time. Until
> now, this NoClassDefFoundError wasn't linked in any way to the root
> cause of the problem (i.e. the first ExceptionInInitializerError
> together with the chained exception that happened during the execution
> of the static initializer). With this change, the NoClassDefFoundError
> will now chain the initial ExceptionInInitializerError as cause,
> making it much easier to detect the problem which lead to the
> NoClassDefFoundError.
> Following is an example from the new JTreg tests which comes which
> this change to demonstrate the feature. Until know, a typical stack
> trace from a NoClassDefFoundError looked as follows:
> java.lang.NoClassDefFoundError: Could not initialize class
> NoClassDefFound$ClassWithFailedInitializer
>      at java.base/java.lang.Class.forName0(Native Method)
>      at java.base/java.lang.Class.forName(Class.java:291)
>      at NoClassDefFound.main(NoClassDefFound.java:38)
> With this change, the same stack trace now looks as follows:
> java.lang.NoClassDefFoundError: Could not initialize class
> NoClassDefFound$ClassWithFailedInitializer
>      at java.base/java.lang.Class.forName0(Native Method)
>      at java.base/java.lang.Class.forName(Class.java:315)
>      at NoClassDefFound.main(NoClassDefFound.java:38)
> Caused by: java.lang.ExceptionInInitializerError
>      at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native
> Method)
>      at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
>      at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
>      at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
>      at java.base/java.lang.Class.newInstance(Class.java:584)
>      at NoClassDefFound$ClassWithFailedInitializer.<clinit>(NoClassDefFound.java:20)
>      at java.base/java.lang.Class.forName0(Native Method)
>      at java.base/java.lang.Class.forName(Class.java:315)
>      at NoClassDefFound.main(NoClassDefFound.java:30)
> Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 2 out of
> bounds for length 1
>      at NoClassDefFound$A.<clinit>(NoClassDefFound.java:9)
>      ... 9 more
> As you can see, the reason for the NoClassDefFoundError when accessing
> the class 'NoClassDefFound$ClassWithFailedInitializer' is actually not
> even in the class or its static initializer itself, but in the class
> 'NoClassDefFound$A' which is a base class of
> 'NoClassDefFound$ClassWithFailedInitializer'. This is not easily
> detectible from the old, plain NoClassDefFoundError.
> As I wrote, the only overhead we have with the new implementation is
> an additional OopHandle field per ClassLoaderData which I think is
> acceptable. The Hashtable object itself is only created lazily, after
> the first occurrence of an ExceptionInInitializerError in the
> corresponding class loader. The whole Hashtable creation and
> storing/quering of ExceptionInInitializerErrors in
> ClassLoaderData::record_init_exception()/ClassLoaderData::query_init_exception()
> is optional in the sense that any errors/exceptions occurring during
> the execution of these functions are ignored and cleared.
> Finally, we also take care to recursively convert all native
> backtraces in the stored ExceptionInInitializerErrors (and their
> suppressed and chained exceptions) into symbolic stack traces in order
> to avoid holding references to classes and prevent their unloading.
> This is implemented in the new private, static method
> java.lang.Throwable::removeNativeBacktrace() which is called for each
> ExceptionInInitializerError before it is stored in the Hashtable.
> Thank you and best regards,
> Volker
> [1] http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2018-June/028310.html

More information about the hotspot-runtime-dev mailing list