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