On Mon, 5 Apr 2021 20:27:53 GMT, Harold Seigel <hsei...@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
