RFR(M): 8203826: Chain class initialization exceptions into later NoClassDefFoundErrors
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....
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 say. 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[] { 119 ClassWithFailedInitializer.class.getProtectionDomain().getCodeSource().getLocation() }, 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....
Hi Volker, This doesn't really address any of the concerns I had with the original proposal - it just moves the "field" from the Java side to the VM side. There is still a massive amount of Java code execution in relation to this - which itself may encounter secondary exceptions. It's very hard to tell if you will leave things in a suitable state if such exceptions arise. My position remains that the primary place to deal with the initialization error is when initialization occurs and the error happens. Subsequent attempted uses of the erroneous class may benefit from some additional information about the nature of the original exceptions, but I don't think full stacktraces are necessary or desirable (and I do believe they will confuse most users given the lack of continuity in the stack frames and that they may have happened in a different thread!). That aside ... There appears to a race on constructing the Hashtable. At least it was not obvious to me where a lock may be held during that process. I can't determine that clearing backtrace in removeNativeBacktrace is correct with respect to the overall protocol within Throwable for dealing with backtrace and stackTrace. I have to wonder why nothing in Throwable clears the backtrace today ? I'm not clear why you record the ExceptionInInitializerError wrapper instead of the actual exception that occurred? Throwable states: + * This method is currently only called from the VM for instances of + * ExceptionInInitializerError which are stored for later chaining into a + * NoClassDefFoundError in order to prevent keeping classes from the native + * backtrace alive. + */ but IIUC it will also be called for instances of Error that occurred which do not get wrapped in EIIE. Regards, David ------ On 30/06/2018 12:53 AM, 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....
Hi, Volker, thanks for improving on my original change and implementing David's and Karen's proposals. David, I think the change addresses a row of your concerns.
proposal - it just moves the "field" from the Java side to the VM side. No, the space overhead in case of successful initialization is reduced from O(classes) to O(classloaders).
There is only runtime overhead if there was an error, and that should be acceptable.
dealing with backtrace and stackTrace. I have to wonder why nothing in Throwable clears the backtrace today ? Maybe the concern about the backTraces is pointless and the conversion to stackTraces should be dropped. As you say, it's done nowhere else, and other backTraces should cause similar issues.
I'm not clear why you record the ExceptionInInitializerError wrapper instead of the actual exception that occurred? Keeping the ExceptionInInitializerError is helpful for people to understand that this has happened during initialization and not directly where the NCDFE is thrown. They will understand that this might have happened in another thread.
Best regards, Goetz.
-----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of David Holmes Sent: Sonntag, 1. Juli 2018 23:48 To: Volker Simonis <volker.simonis@gmail.com>; hotspot-runtime- dev@openjdk.java.net runtime <hotspot-runtime-dev@openjdk.java.net>; Java Core Libs <core-libs-dev@openjdk.java.net> Subject: Re: RFR(M): 8203826: Chain class initialization exceptions into later NoClassDefFoundErrors
Hi Volker,
This doesn't really address any of the concerns I had with the original proposal - it just moves the "field" from the Java side to the VM side. There is still a massive amount of Java code execution in relation to this - which itself may encounter secondary exceptions. It's very hard to tell if you will leave things in a suitable state if such exceptions arise.
My position remains that the primary place to deal with the initialization error is when initialization occurs and the error happens. Subsequent attempted uses of the erroneous class may benefit from some additional information about the nature of the original exceptions, but I don't think full stacktraces are necessary or desirable (and I do believe they will confuse most users given the lack of continuity in the stack frames and that they may have happened in a different thread!).
That aside ...
There appears to a race on constructing the Hashtable. At least it was not obvious to me where a lock may be held during that process.
I can't determine that clearing backtrace in removeNativeBacktrace is correct with respect to the overall protocol within Throwable for dealing with backtrace and stackTrace. I have to wonder why nothing in Throwable clears the backtrace today ?
I'm not clear why you record the ExceptionInInitializerError wrapper instead of the actual exception that occurred?
Throwable states:
+ * This method is currently only called from the VM for instances of + * ExceptionInInitializerError which are stored for later chaining into a + * NoClassDefFoundError in order to prevent keeping classes from the native + * backtrace alive. + */
but IIUC it will also be called for instances of Error that occurred which do not get wrapped in EIIE.
Regards, David ------
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.newInstan ce(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:2
On 30/06/2018 12:53 AM, Volker Simonis wrote: 0)
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_exce ption()
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
Hi Volker, It occurred to me that getting rid of backtrace-s of cause(s)/suppressed exception(s) might not be enough to prevent ClassLoader leaks... On 07/04/2018 10:21 AM, Lindenmaier, Goetz wrote:
dealing with backtrace and stackTrace. I have to wonder why nothing in Throwable clears the backtrace today ? Maybe the concern about the backTraces is pointless and the conversion to stackTraces should be dropped. As you say, it's done nowhere else, and other backTraces should cause similar issues.
Exception objects are typically not retained for longer periods. They are normally caught, dumped to log and let gone. This change retains exception(s) so that they are reachable from a ClassLoader that loaded the failed class. It could happen that the chain of cause(s)/suppressed exception(s) of some ExceptionInInitializerError is an exception object of a class that is loaded by some child ClassLoader of the ClassLoader that loaded the failed class. Such child ClassLoader would have leaked. The solution would be to replace the chain of cause(s)/suppressed exception(s) with a chain of replacement exception objects like this one (this would also take care of backtraces of original exceptions as it would not retain the original exceptions at all): /** * A {@link RuntimeException} that acts as a substitute for the original exception * (checked or unchecked) and mimics the original exception in every aspect except it's type. */ public class ExceptionSubstitute extends RuntimeException { private static final long serialVersionUID = 1; private String originalExceptionClassName, localizedMessage; public ExceptionSubstitute(Throwable originalException) { super(originalException.getMessage()); this.originalExceptionClassName = originalException.getClass().getName(); this.localizedMessage = originalException.getLocalizedMessage(); // substitute originalException's cause Throwable cause = originalException.getCause(); initCause(cause == null ? null : new ExceptionSubstitute(cause)); // substitute originalException's suppressed exceptions if any for (Throwable suppressed : originalException.getSuppressed()) { addSuppressed(new ExceptionSubstitute(suppressed)); } // inherit stack trace elements from originalException setStackTrace(originalException.getStackTrace()); } @Override public Throwable fillInStackTrace() { // don't need our backtrace - will inherit stack trace elements from originalException return this; } @Override public String getLocalizedMessage() { return localizedMessage; } /** * @return the class name of the original exception for which this exception is a substitute */ public String getOriginalExceptionClassName() { return originalExceptionClassName; } /** * Emulate toString() method as if called upon originalException */ @Override public String toString() { String message = getLocalizedMessage(); return (message != null) ? (getOriginalExceptionClassName() + ": " + message) : getOriginalExceptionClassName(); } } Regards, Peter
On 4/07/2018 6:21 PM, Lindenmaier, Goetz wrote:
Hi,
Volker, thanks for improving on my original change and implementing David's and Karen's proposals.
David, I think the change addresses a row of your concerns.
proposal - it just moves the "field" from the Java side to the VM side. No, the space overhead in case of successful initialization is reduced from O(classes) to O(classloaders).
I don't think space usage was one of my concerns.
There is only runtime overhead if there was an error, and that should be acceptable.
dealing with backtrace and stackTrace. I have to wonder why nothing in Throwable clears the backtrace today ? Maybe the concern about the backTraces is pointless and the conversion to stackTraces should be dropped. As you say, it's done nowhere else, and other backTraces should cause similar issues.
My comment on the clearing of Throwable.backtrace was purely in relation to the internal state protocol of Throwable, not about the conversion process as such. The conversion process is needed, as previously discussed, to avoid class/classloader leaks. Exceptions are normally thread-contained so there is limited scope for creating class/classloader leaks. However as soon as you allow this original exception to be stored and potentially later accessed in a different thread then the leak is possible - it might even be possible to hit some kind of loader constraint error if a same named type can be loaded by different classloaders used by the different threads. Add some extra information about the type of the original exception by all means, but not the stacktrace (unless you've stringified it). As I say the place the stack is of interest in when the original exception is encountered. If something is swallowing this then logging is the tool to expose it IMHO.
I'm not clear why you record the ExceptionInInitializerError wrapper instead of the actual exception that occurred? Keeping the ExceptionInInitializerError is helpful for people to understand that this has happened during initialization and not directly where the NCDFE is thrown. They will understand that this might have happened in another thread.
The NCDFE message already states something like "cannot initialize class because prior initialization attempt failed", which I think makes it quite clear. I dispute "they will understand this might have happened in another thread". Cheers, David
Best regards, Goetz.
-----Original Message----- From: core-libs-dev [mailto:core-libs-dev-bounces@openjdk.java.net] On Behalf Of David Holmes Sent: Sonntag, 1. Juli 2018 23:48 To: Volker Simonis <volker.simonis@gmail.com>; hotspot-runtime- dev@openjdk.java.net runtime <hotspot-runtime-dev@openjdk.java.net>; Java Core Libs <core-libs-dev@openjdk.java.net> Subject: Re: RFR(M): 8203826: Chain class initialization exceptions into later NoClassDefFoundErrors
Hi Volker,
This doesn't really address any of the concerns I had with the original proposal - it just moves the "field" from the Java side to the VM side. There is still a massive amount of Java code execution in relation to this - which itself may encounter secondary exceptions. It's very hard to tell if you will leave things in a suitable state if such exceptions arise.
My position remains that the primary place to deal with the initialization error is when initialization occurs and the error happens. Subsequent attempted uses of the erroneous class may benefit from some additional information about the nature of the original exceptions, but I don't think full stacktraces are necessary or desirable (and I do believe they will confuse most users given the lack of continuity in the stack frames and that they may have happened in a different thread!).
That aside ...
There appears to a race on constructing the Hashtable. At least it was not obvious to me where a lock may be held during that process.
I can't determine that clearing backtrace in removeNativeBacktrace is correct with respect to the overall protocol within Throwable for dealing with backtrace and stackTrace. I have to wonder why nothing in Throwable clears the backtrace today ?
I'm not clear why you record the ExceptionInInitializerError wrapper instead of the actual exception that occurred?
Throwable states:
+ * This method is currently only called from the VM for instances of + * ExceptionInInitializerError which are stored for later chaining into a + * NoClassDefFoundError in order to prevent keeping classes from the native + * backtrace alive. + */
but IIUC it will also be called for instances of Error that occurred which do not get wrapped in EIIE.
Regards, David ------
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.newInstan ce(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:2
On 30/06/2018 12:53 AM, Volker Simonis wrote: 0)
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_exce ption()
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
Hi, On 07/05/2018 01:01 AM, David Holmes wrote:
I dispute "they will understand this might have happened in another thread".
What if the stack trace was like the following... Before patch: 1st attempt [ForkJoinPool.commonPool-worker-3]: java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:20) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728) at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020) at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594) at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177) Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:12) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more 2nd attempt [ForkJoinPool.commonPool-worker-5]: java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at ClinitFailure.lambda$main$1(ClinitFailure.java:28) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783) at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479) at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020) at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594) at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177) After patch: 1st attempt [ForkJoinPool.commonPool-worker-3]: java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728) at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020) at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594) at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177) Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more 2nd attempt [ForkJoinPool.commonPool-worker-5]: java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at java.base/java.lang.ClassLoader.throwReinitException(ClassLoader.java:3062) at ClinitFailure.lambda$main$1(ClinitFailure.java:25) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783) at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479) at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020) at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594) at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177) Caused by: java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3 at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728) ... 5 more Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more This is what gets printed by the sample program: public class ClinitFailure { static class Faulty { static { try { int i = (new int[0])[1]; } catch (Exception e) { throw new RuntimeException("Can't get it!", e); } } } public static void main(String[] args) throws Exception { CompletableFuture.runAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n1st attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).thenRunAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n2nd attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).join(); } } When the following patch is applied: http://cr.openjdk.java.net/~plevart/jdk-dev/8203826_NoClassDefFoundError.cau... I took Volker's patch and modified it a bit: - The logic to construct and throw NoClassDefFoundError and to record initial <clinit> exception is in java now. It uses ClassLoaderValue internal API to save the chains of exception(s) for faulty classes. It is easier to do such logic in Java and less error prone. - The chain of original <clinit> exception(s) is replaced with substitutes that mimic .toString() and .printStackTrace() methods of original chain, but don't reference any classes outside bootstrap class loader - The replacement chain of original exceptions adds a custom message insert into the top exception as a hint to the user: java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3 So, what do you think of this one? Regards, Peter
Hi Peter, On 7/07/2018 2:10 AM, Peter Levart wrote:
Hi,
On 07/05/2018 01:01 AM, David Holmes wrote:
I dispute "they will understand this might have happened in another thread".
What if the stack trace was like the following...
Yes your suggestion makes it much clearer. But ... my whole objection here is doing all this extraneous execution of Java code in response to the initial exception. The more Java code we execute the more likely we will hit secondary exceptions and the greater the possibility of unintended interactions that might lead back to the class that can't be initialized. I just don't think this level of effort is warranted. Cheers, David -----
Before patch:
1st attempt [ForkJoinPool.commonPool-worker-3]:
java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:20) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:12) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more
2nd attempt [ForkJoinPool.commonPool-worker-5]:
java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at ClinitFailure.lambda$main$1(ClinitFailure.java:28) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
After patch:
1st attempt [ForkJoinPool.commonPool-worker-3]:
java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more
2nd attempt [ForkJoinPool.commonPool-worker-5]:
java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at java.base/java.lang.ClassLoader.throwReinitException(ClassLoader.java:3062) at ClinitFailure.lambda$main$1(ClinitFailure.java:25) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3 at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
... 5 more Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more
This is what gets printed by the sample program:
public class ClinitFailure {
static class Faulty { static { try { int i = (new int[0])[1]; } catch (Exception e) { throw new RuntimeException("Can't get it!", e); } } }
public static void main(String[] args) throws Exception { CompletableFuture.runAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n1st attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).thenRunAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n2nd attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).join(); } }
When the following patch is applied:
http://cr.openjdk.java.net/~plevart/jdk-dev/8203826_NoClassDefFoundError.cau...
I took Volker's patch and modified it a bit:
- The logic to construct and throw NoClassDefFoundError and to record initial <clinit> exception is in java now. It uses ClassLoaderValue internal API to save the chains of exception(s) for faulty classes. It is easier to do such logic in Java and less error prone. - The chain of original <clinit> exception(s) is replaced with substitutes that mimic .toString() and .printStackTrace() methods of original chain, but don't reference any classes outside bootstrap class loader - The replacement chain of original exceptions adds a custom message insert into the top exception as a hint to the user:
java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3
So, what do you think of this one?
Regards, Peter
Hi David, On 07/09/2018 03:37 AM, David Holmes wrote:
Hi Peter,
On 7/07/2018 2:10 AM, Peter Levart wrote:
Hi,
On 07/05/2018 01:01 AM, David Holmes wrote:
I dispute "they will understand this might have happened in another thread".
What if the stack trace was like the following...
Yes your suggestion makes it much clearer.
But ... my whole objection here is doing all this extraneous execution of Java code in response to the initial exception. The more Java code we execute the more likely we will hit secondary exceptions and the greater the possibility of unintended interactions that might lead back to the class that can't be initialized. I just don't think this level of effort is warranted. I
I agree that more classes are involved, but they are all JDK classes and their number is constant. Meaning, if they are OK and don't fail when initializing, there's no danger of unintended interactions that would be caused by initialization errors in other classes. And even if those additional needed classes had problems in their initialization, I think that the consequences would be under control. Let's see what additional classes are needed when the presented patch is used as opposed to classes needed in current logic: In step 7, when super class/interface initialization fails and in steps 10/11 when the class initialization fails, we record the error thrown (record_init_exception). In addition to previously needed classes we also need: - ClassLoader, - ClassLoader$InitExceptionSubst (with dependencies: RuntimeException, Exception), - ClassLoaderValue (with dependencies: AbstractClassLoaderValue, AbstractClassLoaderValue$Sub, AbstractClassLoaderValue$Memoizer, ConcurrentHashMap + deps) When we throw NoClassDefFoundError, we don't need any other additional classes that wouldn't already be needed originally. So I can see that when above additional classes had problems initializing themselves, there would be errors thrown from their usage when recording initial initialization exception of some unrelated class, but such errors would be ignored (step 7): 979 // Record the exception thrown from super class/interface initialization so that 980 // it can be chained into potential later NoClassDefFoundErrors. 981 class_loader_data()->record_init_exception(java_mirror_handle(), e, THREAD); 982 // Locks object, set state, and notify all waiting threads 983 set_initialization_state_and_notify(initialization_error, THREAD); 984 *CLEAR_PENDING_EXCEPTION*; steps 10/11: 1037 // Record the exception that originally caused <clinit> to fail so 1038 // it can be chained into potential later NoClassDefFoundErrors. 1039 class_loader_data()->record_init_exception(java_mirror_handle(), e, THREAD); 1040 // Locks object, set state, and notify all waiting threads 1041 set_initialization_state_and_notify(initialization_error, THREAD); 1042 *CLEAR_PENDING_EXCEPTION*; It might be that those ignored exceptions would cause later use of those additional classes to throw NoClassDefFoundError instead of ExceptionInInitializerError (depending on whether it was an initial attempt to initialize those additional classes or not), but I can't see any other undesirable consequence. Do you? I'll try to provoke initialization errors in those additional classes to see what happens. Will get back when I have results of the experiment... Regards, Peter P.S. Executing java code as part of VM logic plays well in Jigsaw for example. If there is an acceptable fallback in case of java logic failure, everything seems to be OK.
Cheers, David -----
Before patch:
1st attempt [ForkJoinPool.commonPool-worker-3]:
java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:20) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:12) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more
2nd attempt [ForkJoinPool.commonPool-worker-5]:
java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at ClinitFailure.lambda$main$1(ClinitFailure.java:28) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
After patch:
1st attempt [ForkJoinPool.commonPool-worker-3]:
java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more
2nd attempt [ForkJoinPool.commonPool-worker-5]:
java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at java.base/java.lang.ClassLoader.throwReinitException(ClassLoader.java:3062) at ClinitFailure.lambda$main$1(ClinitFailure.java:25) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3 at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
... 5 more Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more
This is what gets printed by the sample program:
public class ClinitFailure {
static class Faulty { static { try { int i = (new int[0])[1]; } catch (Exception e) { throw new RuntimeException("Can't get it!", e); } } }
public static void main(String[] args) throws Exception { CompletableFuture.runAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n1st attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).thenRunAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n2nd attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).join(); } }
When the following patch is applied:
http://cr.openjdk.java.net/~plevart/jdk-dev/8203826_NoClassDefFoundError.cau...
I took Volker's patch and modified it a bit:
- The logic to construct and throw NoClassDefFoundError and to record initial <clinit> exception is in java now. It uses ClassLoaderValue internal API to save the chains of exception(s) for faulty classes. It is easier to do such logic in Java and less error prone. - The chain of original <clinit> exception(s) is replaced with substitutes that mimic .toString() and .printStackTrace() methods of original chain, but don't reference any classes outside bootstrap class loader - The replacement chain of original exceptions adds a custom message insert into the top exception as a hint to the user:
java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3
So, what do you think of this one?
Regards, Peter
On 9/07/2018 5:22 PM, Peter Levart wrote:
Hi David,
On 07/09/2018 03:37 AM, David Holmes wrote:
Hi Peter,
On 7/07/2018 2:10 AM, Peter Levart wrote:
Hi,
On 07/05/2018 01:01 AM, David Holmes wrote:
I dispute "they will understand this might have happened in another thread".
What if the stack trace was like the following...
Yes your suggestion makes it much clearer.
But ... my whole objection here is doing all this extraneous execution of Java code in response to the initial exception. The more Java code we execute the more likely we will hit secondary exceptions and the greater the possibility of unintended interactions that might lead back to the class that can't be initialized. I just don't think this level of effort is warranted. I
I agree that more classes are involved, but they are all JDK classes and their number is constant. Meaning, if they are OK and don't fail when initializing, there's no danger of unintended interactions that would be caused by initialization errors in other classes. And even if those additional needed classes had problems in their initialization, I think that the consequences would be under control.
That's not what I mean. I'm not concerned about circular initialization failures due to failing to initialize the classes used in this "hook". I'm concerned about the overall amount of Java code execution that this involves, which may trigger other exceptions (e.g. OOME) and which may incur additional logging or event generation that may in turn interact in some way with the original class being initialized. I just think this is complete overkill for addressing the perceived problem. David -----
Let's see what additional classes are needed when the presented patch is used as opposed to classes needed in current logic:
In step 7, when super class/interface initialization fails and in steps 10/11 when the class initialization fails, we record the error thrown (record_init_exception). In addition to previously needed classes we also need: - ClassLoader, - ClassLoader$InitExceptionSubst (with dependencies: RuntimeException, Exception), - ClassLoaderValue (with dependencies: AbstractClassLoaderValue, AbstractClassLoaderValue$Sub, AbstractClassLoaderValue$Memoizer, ConcurrentHashMap + deps)
When we throw NoClassDefFoundError, we don't need any other additional classes that wouldn't already be needed originally.
So I can see that when above additional classes had problems initializing themselves, there would be errors thrown from their usage when recording initial initialization exception of some unrelated class, but such errors would be ignored (step 7):
979 // Record the exception thrown from super class/interface initialization so that 980 // it can be chained into potential later NoClassDefFoundErrors. 981 class_loader_data()->record_init_exception(java_mirror_handle(), e, THREAD); 982 // Locks object, set state, and notify all waiting threads 983 set_initialization_state_and_notify(initialization_error, THREAD); 984 *CLEAR_PENDING_EXCEPTION*;
steps 10/11:
1037 // Record the exception that originally caused <clinit> to fail so 1038 // it can be chained into potential later NoClassDefFoundErrors. 1039 class_loader_data()->record_init_exception(java_mirror_handle(), e, THREAD); 1040 // Locks object, set state, and notify all waiting threads 1041 set_initialization_state_and_notify(initialization_error, THREAD); 1042 *CLEAR_PENDING_EXCEPTION*;
It might be that those ignored exceptions would cause later use of those additional classes to throw NoClassDefFoundError instead of ExceptionInInitializerError (depending on whether it was an initial attempt to initialize those additional classes or not), but I can't see any other undesirable consequence. Do you?
I'll try to provoke initialization errors in those additional classes to see what happens. Will get back when I have results of the experiment...
Regards, Peter
P.S.
Executing java code as part of VM logic plays well in Jigsaw for example. If there is an acceptable fallback in case of java logic failure, everything seems to be OK.
Cheers, David -----
Before patch:
1st attempt [ForkJoinPool.commonPool-worker-3]:
java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:20) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:12) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more
2nd attempt [ForkJoinPool.commonPool-worker-5]:
java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at ClinitFailure.lambda$main$1(ClinitFailure.java:28) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
After patch:
1st attempt [ForkJoinPool.commonPool-worker-3]:
java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more
2nd attempt [ForkJoinPool.commonPool-worker-5]:
java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at java.base/java.lang.ClassLoader.throwReinitException(ClassLoader.java:3062) at ClinitFailure.lambda$main$1(ClinitFailure.java:25) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656) at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3 at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
... 5 more Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more
This is what gets printed by the sample program:
public class ClinitFailure {
static class Faulty { static { try { int i = (new int[0])[1]; } catch (Exception e) { throw new RuntimeException("Can't get it!", e); } } }
public static void main(String[] args) throws Exception { CompletableFuture.runAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n1st attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).thenRunAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n2nd attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).join(); } }
When the following patch is applied:
http://cr.openjdk.java.net/~plevart/jdk-dev/8203826_NoClassDefFoundError.cau...
I took Volker's patch and modified it a bit:
- The logic to construct and throw NoClassDefFoundError and to record initial <clinit> exception is in java now. It uses ClassLoaderValue internal API to save the chains of exception(s) for faulty classes. It is easier to do such logic in Java and less error prone. - The chain of original <clinit> exception(s) is replaced with substitutes that mimic .toString() and .printStackTrace() methods of original chain, but don't reference any classes outside bootstrap class loader - The replacement chain of original exceptions adds a custom message insert into the top exception as a hint to the user:
java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3
So, what do you think of this one?
Regards, Peter
Hi David, On 07/09/2018 09:33 AM, David Holmes wrote:
On 9/07/2018 5:22 PM, Peter Levart wrote:
Hi David,
On 07/09/2018 03:37 AM, David Holmes wrote:
Hi Peter,
On 7/07/2018 2:10 AM, Peter Levart wrote:
Hi,
On 07/05/2018 01:01 AM, David Holmes wrote:
I dispute "they will understand this might have happened in another thread".
What if the stack trace was like the following...
Yes your suggestion makes it much clearer.
But ... my whole objection here is doing all this extraneous execution of Java code in response to the initial exception. The more Java code we execute the more likely we will hit secondary exceptions and the greater the possibility of unintended interactions that might lead back to the class that can't be initialized. I just don't think this level of effort is warranted. I
I agree that more classes are involved, but they are all JDK classes and their number is constant. Meaning, if they are OK and don't fail when initializing, there's no danger of unintended interactions that would be caused by initialization errors in other classes. And even if those additional needed classes had problems in their initialization, I think that the consequences would be under control.
That's not what I mean. I'm not concerned about circular initialization failures due to failing to initialize the classes used in this "hook". I'm concerned about the overall amount of Java code execution that this involves, which may trigger other exceptions (e.g. OOME) and which may incur additional logging or event generation that may in turn interact in some way with the original class being initialized.
I still can't see what you see. If java code that records initial initialization error throws OOME, it will be ignored and initial exception will not be recorded. Later NoClassDefFoundError would not contain the nice chain of causes, but that's the only undesirable consequence. The code doesn't log OOME, it simply ignores it. If those additional exceptions cause any events on the VM level and those events interact in some way with the original class being initialized, this interaction will fail, but such interaction with original class would fail even if it was caused by anything else, because it is the original class that had problems initializing itself in the 1st place.
I just think this is complete overkill for addressing the perceived problem.
Perhaps. But it would be nice to have. Regards, Peter
David -----
Let's see what additional classes are needed when the presented patch is used as opposed to classes needed in current logic:
In step 7, when super class/interface initialization fails and in steps 10/11 when the class initialization fails, we record the error thrown (record_init_exception). In addition to previously needed classes we also need: - ClassLoader, - ClassLoader$InitExceptionSubst (with dependencies: RuntimeException, Exception), - ClassLoaderValue (with dependencies: AbstractClassLoaderValue, AbstractClassLoaderValue$Sub, AbstractClassLoaderValue$Memoizer, ConcurrentHashMap + deps)
When we throw NoClassDefFoundError, we don't need any other additional classes that wouldn't already be needed originally.
So I can see that when above additional classes had problems initializing themselves, there would be errors thrown from their usage when recording initial initialization exception of some unrelated class, but such errors would be ignored (step 7):
979 // Record the exception thrown from super class/interface initialization so that 980 // it can be chained into potential later NoClassDefFoundErrors. 981 class_loader_data()->record_init_exception(java_mirror_handle(), e, THREAD); 982 // Locks object, set state, and notify all waiting threads 983 set_initialization_state_and_notify(initialization_error, THREAD); 984 *CLEAR_PENDING_EXCEPTION*;
steps 10/11:
1037 // Record the exception that originally caused <clinit> to fail so 1038 // it can be chained into potential later NoClassDefFoundErrors. 1039 class_loader_data()->record_init_exception(java_mirror_handle(), e, THREAD); 1040 // Locks object, set state, and notify all waiting threads 1041 set_initialization_state_and_notify(initialization_error, THREAD); 1042 *CLEAR_PENDING_EXCEPTION*;
It might be that those ignored exceptions would cause later use of those additional classes to throw NoClassDefFoundError instead of ExceptionInInitializerError (depending on whether it was an initial attempt to initialize those additional classes or not), but I can't see any other undesirable consequence. Do you?
I'll try to provoke initialization errors in those additional classes to see what happens. Will get back when I have results of the experiment...
Regards, Peter
P.S.
Executing java code as part of VM logic plays well in Jigsaw for example. If there is an acceptable fallback in case of java logic failure, everything seems to be OK.
Cheers, David -----
Before patch:
1st attempt [ForkJoinPool.commonPool-worker-3]:
java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:20) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:12) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more
2nd attempt [ForkJoinPool.commonPool-worker-5]:
java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at ClinitFailure.lambda$main$1(ClinitFailure.java:28) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
After patch:
1st attempt [ForkJoinPool.commonPool-worker-3]:
java.lang.ExceptionInInitializerError at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more
2nd attempt [ForkJoinPool.commonPool-worker-5]:
java.lang.NoClassDefFoundError: Could not initialize class ClinitFailure$Faulty at java.base/java.lang.ClassLoader.throwReinitException(ClassLoader.java:3062) at ClinitFailure.lambda$main$1(ClinitFailure.java:25) at java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
at java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3 at ClinitFailure.lambda$main$0(ClinitFailure.java:18) at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
... 5 more Caused by: java.lang.RuntimeException: Can't get it! at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:10) ... 8 more Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 0 at ClinitFailure$Faulty.<clinit>(ClinitFailure.java:8) ... 8 more
This is what gets printed by the sample program:
public class ClinitFailure {
static class Faulty { static { try { int i = (new int[0])[1]; } catch (Exception e) { throw new RuntimeException("Can't get it!", e); } } }
public static void main(String[] args) throws Exception { CompletableFuture.runAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n1st attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).thenRunAsync(() -> { try { new Faulty(); } catch (Throwable e) { System.out.printf("\n2nd attempt [%s]:\n\n", Thread.currentThread().getName()); e.printStackTrace(System.out); } }).join(); } }
When the following patch is applied:
http://cr.openjdk.java.net/~plevart/jdk-dev/8203826_NoClassDefFoundError.cau...
I took Volker's patch and modified it a bit:
- The logic to construct and throw NoClassDefFoundError and to record initial <clinit> exception is in java now. It uses ClassLoaderValue internal API to save the chains of exception(s) for faulty classes. It is easier to do such logic in Java and less error prone. - The chain of original <clinit> exception(s) is replaced with substitutes that mimic .toString() and .printStackTrace() methods of original chain, but don't reference any classes outside bootstrap class loader - The replacement chain of original exceptions adds a custom message insert into the top exception as a hint to the user:
java.lang.ExceptionInInitializerError: 11 ms ago in thread ForkJoinPool.commonPool-worker-3
So, what do you think of this one?
Regards, Peter
participants (4)
-
David Holmes
-
Lindenmaier, Goetz
-
Peter Levart
-
Volker Simonis