RFR: 8264711: More runtime TRAPS cleanups [v2]

[David Holmes]

On Mon, 5 Apr 2021 20:27:53 GMT, Harold Seigel <hseigel at openjdk.org> wrote:

>> Please review this additional cleanup of use of TRAPS in hotspot runtime code.  The changes were tested with Mach5 tiers 1-2 on Linux, Mac OS, and Windows and Mach5 tiers 3-5 on Linux x64.
>> 
>> Thanks, Harold
>
> Harold Seigel has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Undo change to ObjectSynchronizer::jni_exit()

Hi Harold,

Lots of good cleanup here but also a couple of things that I think are incorrect. Platform string creation can throw exceptions; and I also believe the ClassFileLoadHook can too.

Thanks,
David

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

> 394: 
> 395: // Converts a C string to a Java String based on current encoding
> 396: Handle java_lang_String::create_from_platform_dependent_str(JavaThread* current, const char* str) {

This change is incorrect. JNU_NewStringPlatform can raise an exception so TRAPS here is correct.

src/hotspot/share/classfile/klassFactory.cpp line 117:

> 115:                                                    Handle protection_domain,
> 116:                                                    JvmtiCachedClassFileData** cached_class_file,
> 117:                                                    TRAPS) {

I don't think this removal of TRAPS is correct. The  ClassFileLoadHook could result in an exception becoming pending.

src/hotspot/share/prims/jvmtiEnv.cpp line 709:

> 707: 
> 708:     // need the path as java.lang.String
> 709:     Handle path = java_lang_String::create_from_platform_dependent_str(THREAD, segment);

This change will need reverting as an exception is possible as previously noted.

But note that if there was no possibility of exception here then the following check of HAS_PENDING_EXCEPTION should also have been removed.

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

Changes requested by dholmes (Reviewer).

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