RFR: 8302491: NoClassDefFoundError omits the original cause of an error [v5]

[David Holmes]

On Mon, 6 Mar 2023 20:28:40 GMT, Ilarion Nakonechnyy <inakonechnyy at openjdk.org> wrote:

>> The proposed approach added a new function for getting the cause of an exception -`java_lang_Throwable::get_cause_simple `, that gets called within `InstanceKlass::add_initialization_error`  if an old one `java_lang_Throwable::get_cause_with_stack_trace` didn't succeed because of an exception during the VM call. The simple function doesn't call the VM for getting a stack trace but fills in any other information about an exception.
>> 
>> Besides that, the discovering information about an exception was added to `ConstantPoolCacheEntry::save_and_throw_indy_exc` function. 
>> 
>> Jtreg for reproducing the issue also was added to the commit. 
>> The commit was tested with tier1 tests.
>
> Ilarion Nakonechnyy has updated the pull request incrementally with one additional commit since the last revision:
> 
>   1/ create_initialization_error(): return empty exception, if
>   EIIE creation failed.
>   2/ remove testcase

Thanks for the updates. Mostly just nits with comments and names.

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

> 2739: 
> 2740:   // Throw ExceptionInInitializerError as the cause with this exception in
> 2741:   // the message and stack trace.

I thought I'd flagged this earlier - this comment is not correct. Suggestion for a method-level comment (which allows removing a comment further down).

// Creates an ExceptionInInitializerError to be recorded as the initialization error when class initialization
// failed due to the passed in `throwable`. We cannot save `throwable` directly due to issues with keeping alive
// all objects referenced via its stacktrace. So instead we save a new EIIE instance, with the same message and 
// symbolic stacktrace of `throwable`.

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

> 2754: 
> 2755:   Symbol* exception_name = vmSymbols::java_lang_ExceptionInInitializerError();
> 2756:   Handle h_cause = Exceptions::new_exception(current, exception_name, st.as_string());

Existing: h_cause in an inappropriate name as this is not the "cause" of anything. Suggest simply h_eiie

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

> 2756:   Handle h_cause = Exceptions::new_exception(current, exception_name, st.as_string());
> 2757:   // If new_exception returns a different exception while creating the exception,
> 2758:   // abandon the attempts to save initialization error and return null.

Nit: save _the_ initialization error ...

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

> 2758:   // abandon the attempts to save initialization error and return null.
> 2759:   // We can't just return an original throwable (that is get passed as a parameter),
> 2760:   // because it would keep all the caller classes alive.

Delete this.

src/hotspot/share/oops/instanceKlass.cpp line 983:

> 981:   // this would be still be helpful.
> 982:   JavaThread* THREAD = current;
> 983:   Handle cause = java_lang_Throwable::create_initialization_error(current, exception);

Nit: s/cause/init_error/

src/hotspot/share/oops/instanceKlass.cpp line 987:

> 985:     CLEAR_PENDING_EXCEPTION;
> 986:     return;
> 987:   }

We still need to return if we got null.

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

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12566