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

[David Holmes]

On Tue, 21 Feb 2023 12:12:56 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 two additional commits since the last revision:
> 
>  - Get rid of redundant code -
>    merge get_cause_with_stack_trace and get_cause_simple
>  - Review corrections

I've put a long comment in the JBS issue explaining the process of saving the "initialization error" in detail.

Further changes in relation to secondary exception handling is needed - see below.

Some other issues also raised.

Thanks.

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

> 2762:                           h_cause->klass()->external_name());
> 2763:       return Handle();
> 2764:   }

You need to do this whether `with_stack_trace` or not, otherwise you will install the exception from `new_exception` as the initialization error for this class, when in fact it was not the initialization error at all.

src/hotspot/share/classfile/javaClasses.hpp line 618:

> 616: 
> 617:   // For recreating class initialization error exceptions.
> 618:   static Handle get_cause(Handle throwable, bool with_stack_trace, TRAPS);

Existing: this is an absolutely terrible name for this method! Given that Throwable's have a "cause" this method would be assumed to relate to getting the cause of the passed in `throwable`, but it has nothing to do with that at all! This method is about creating an `ExceptionInInitializerError` instance that includes information from `throwable` (including class name, message and stacktrace).
I suggest, based on terminology in `instanceKlass` that this be called `create_initialization_error`.

src/hotspot/share/oops/cpCache.cpp line 476:

> 474:   Symbol* message = java_lang_Throwable::detail_message(PENDING_EXCEPTION);
> 475:   // Try to discover the cause
> 476:   oop cause = java_lang_Throwable::cause(PENDING_EXCEPTION);

This enhancement is unrelated to the current JBS issue. I think it a good enhancement but it should be done separately.

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

Changes requested by dholmes (Reviewer).

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