RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

[Daniel D. Daugherty]

On 9/5/19 1:36 AM, David Holmes wrote:
> Hi Dan,
>
> With my CSR Group member hat on ....
>
> On 5/09/2019 8:06 am, Daniel D. Daugherty wrote:
>> Brent,
>>
>> You currently have '-XX:+ClassForNameDeferLinking' as a 'product' 
>> option, but
>> product options are harder to remove down the road. Would it be 
>> better as a
>> diagnostic option? A diagnostic option requires 
>
> Whether a flag is product or diagnostic (or experimental) should be a 
> basic property of the flag based on its purpose. I would not want to 
> see a trend of making new flags diagnostic just because it is easier 
> to get rid of them later. The expectation with this fix and flag 
> (which I've been heavily involved in) is that production code may be 
> impacted by the change and need to use the flag to restore previous 
> behaviour. So it really is a product flag that end users should be 
> comfortable in using if they need 
> it.

This is the key phrase:

 > production code may be impacted by the change and need to
 > use the flag to restore previous behaviour.

Thanks for making that clear. Maybe I missed it in what I read,
but it is now clear that this should be a product flag.

Dan

>
> The removal process for a product flag is phased (deprecate, obsolete, 
> expire) but not particularly onerous. There is an expectation that 
> this flag may be deprecated in 15 as it is intended as a transitional 
> flag.
>
> Thanks,
> David
> -----
>
>> '-XX:+UnlockDiagnosticVMOptions'
>> to be specified before it can be used, e.g.:
>>
>>      java -XX:+UnlockDiagnosticVMOptions 
>> -XX:+ClassForNameDeferLinking Foo
>>
>> so it is a bit harder to use, but maybe that's a Good Thing (TM).
>>
>> Dan
>>
>>
>> On 9/4/19 5:12 PM, Brent Christian wrote:
>>> Hi,
>>>
>>> Please review my fix for JDK-8212117[1].  The webrev is here:
>>>
>>> http://cr.openjdk.java.net/~bchristi/8212117/webrev09/
>>>
>>> There is also a CSR[2] in need of review.
>>>
>>> The spec for the 2-arg and 3-arg Class.forName() methods states they 
>>> will "locate, load, and link" the class, however the linking part is 
>>> not ensured to happen.
>>>
>>> Although the VM spec allows flexibility WRT when classes are linked, 
>>> this is a corner where the Class.forName() spec is not being upheld. 
>>> While this is not an issue for most usages, 8181144 [3] demonstrates 
>>> how this can be a problem (with the debugging interface, in this case).
>>>
>>> This fix ensures that linking happens during the course of 
>>> Class.forName().  Class.forName() already @throws LinkageError, so 
>>> no spec change is needed there. MethodHandles.Lookup.findClass() 
>>> needs a small spec update, due to calling Class.forName with 
>>> init=false,
>>>
>>> Of course Errors are not required to be caught.  It is therefore 
>>> possible that the new behavior could introduce previously unseen, 
>>> possibly unhandled LinkageErrors.  A new VM flag 
>>> (-XX:+ClassForNameDeferLinking) is introduced to restore the 
>>> previous behavior (to keep such code running until it can be updated).
>>>
>>> This change surfaced a couple new "A JNI error has occurred" 
>>> situations (see 8181033[5]) in the Launcher, by way of
>>>   test/jdk/tools/launcher/MainClassCantBeLoadedTest.java
>>> (using the 3-arg Class::forName, detailed in the bug report[4]),
>>> and
>>>   test/jdk/tools/launcher/modules/basic/LauncherErrors.java
>>> (using the 2-arg Class::forName).
>>>
>>> The Launcher is updated to maintain non-confusing error messages :).
>>>
>>> The new test included with this fix ensures that 8181144[3] is 
>>> addressed.  Thanks go to Serguei Spitsyn for writing the test.
>>>
>>> Automated corelibs and hotspot tests pass cleanly.
>>>
>>> Thanks,
>>> -Brent
>>>
>>> -- 
>>> 1. https://bugs.openjdk.java.net/browse/JDK-8212117
>>>
>>> 2. https://bugs.openjdk.java.net/browse/JDK-8222071
>>>
>>> 3. https://bugs.openjdk.java.net/browse/JDK-8181144
>>>
>>> 4. 
>>> https://bugs.openjdk.java.net/browse/JDK-8212117?focusedCommentId=14215986&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14215986 
>>>
>>>
>>> 5. https://bugs.openjdk.java.net/browse/JDK-8181033
>>>
>>