On Mon, 5 Apr 2021 23:28:38 GMT, David Holmes <[email protected]> wrote:
>> 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
Please review this latest version of this change containing reviewer suggested
changes.
Thanks, Harold
> 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.
Thanks for finding this. I reverted this change and the change to
as_platform_dependent_str() because it calls GetStringPlatformChars() which
calls JNU_GetStringPlatformChars() which can throw an exception.
> 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.
Reverted.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3345
