RFR: 8048190: NoClassDefFoundError omits original ExceptionInInitializerError [v8]

[David Holmes]

On Tue, 10 Aug 2021 03:43:09 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This is a change to save the class initialization error stack trace in a hashtable and return it as the cause when NoClassDefFoundError is thrown.  The first commit is a more limited version of this that just changes the message, by adding to the message string.  The second commit is getting and saving the stack trace for the original exception, and using the thread in the message.  See CR for more details about how the message looks.
>> 
>> Tested with tier1-3 tests on 3 platforms.  Tier 4-6 in progress (all but two done and passed).
>
> Coleen Phillimore has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
> 
>  - Make Hashtable a fixed size, not resizable and bigger.
>  - Merge branch 'master' into init-oom
>  - Throw EIIE with the original exception as the cause and stack trace. Much safer. Also remove the redundant test and not try to get OOM because that would make the test unreliable.
>  - Hooray another jasm file gone!
>  - Fix java.base/bootstrap class loader comment.
>  - Create new exception during initialization saving, so only need to save an oop. (Could potentially move to Doug's secret Class location.  If the exception is not bootstrap exception, throw EIIE instead with message of original exception.
>  - Merge branch 'master' into init-oom
>  - David review comments.
>  - Merge branch 'master' into init-oom
>  - 8048190: NoClassDefFoundError omits original ExceptionInInitializerError
>  - ... and 1 more: https://git.openjdk.java.net/jdk/compare/2bf5cbe5...ffec5bbd

Hi Coleen,

Functional changes look good!

A couple of nits with the test.

Thanks,
David

src/hotspot/share/classfile/javaClasses.cpp line 2665:

> 2663:   // the message and stack trace.
> 2664: 
> 2665:   // Now create the same exception with this stacktrace and thread name.

Comment needs updating

test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java line 53:

> 51:     static public class ThrowsSpecialException {
> 52:         static {
> 53:             if (true) foo();

Not sure why you need to define foo() - and if you use foo() you shouldn't need the "if (true)".

test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java line 60:

> 58:     }
> 59: 
> 60:     static public class InitOOM {

Why InitOOM instead of ThrowsOOM?

test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java line 112:

> 110:         int i = 0;
> 111:         for (String className : classNames) {
> 112:             for (int tries = 2; tries--> 0; ) {

Nit: need space after --

test/hotspot/jtreg/runtime/ClassInitErrors/InitExceptionUnloadTest.java line 135:

> 133:     public static void main(java.lang.String[] unused) throws Throwable {
> 134:         test();
> 135:         test();

Why twice?

-------------

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4996