RFR: 8260341: CDS dump VM init code does not check exceptions [v2]

[Coleen Phillimore]

On Wed, 10 Feb 2021 18:51:53 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> When CDS dumping is enabled, special initialization happens during VM init. However, many of these calls do not properly check for exception. Instead, they rely on the implicit knowledge that `metaspace::allocate()` will exit the VM when allocation fails during CDS dumping. This makes the code hard to understand and tightly coupled to `metaspace::allocate()`.
>> 
>> The fix is: all code that makes allocation should be using CHECK macros, so each block of code can be individually understood without considering the behavior of `metaspace::allocate()`.
>> 
>> I added `TRAPS` to a bunch of CDS-related functions that are called during VM init. In some cases, I changed `Thread* THREAD` to `TRAPS`. This also eliminated a few `Thread* THREAD = Thread::current()` calls.
>> 
>> The "root" of these calls, such as `MetaspaceShared::prepare_for_dumping()`, now follows this pattern:
>> 
>>   EXCEPTION_MARK;
>>   ClassLoader::initialize_shared_path(THREAD);
>>   if (HAS_PENDING_EXCEPTION) {
>>     java_lang_Throwable::print(PENDING_EXCEPTION, tty);
>>     vm_exit_during_initialization("ClassLoader::initialize_shared_path() failed unexpectedly");
>>   }
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Changed THREAD in tail calls to CHECK

What Harold said. We use THREAD if the call is the last call in the function because I thought there was a tail call problem with one of the compilers once.  I think this was the bug I was thinking about and it's only in the return statement:
https://bugs.openjdk.java.net/browse/JDK-6889002
If you verified that the HAS_PENDING_EXCEPTION check evaporates, that's fine then, and better to have CHECK.  As you say, someone might add some code after it.

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

Marked as reviewed by coleenp (Reviewer).

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