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

Mon, 09 Jul 2018 00:53:02 -0700 

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.cause/webrev.01/ 


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

Reply via email to