[9 + 8u] RFR - 8066612: Add a test that will call getDeclaredFields() on all classes and try to set them accessible.

[Daniel Fuchs]

On 04/12/14 13:06, David Holmes wrote:
> Hi Daniel,
>
> On 4/12/2014 9:38 PM, Daniel Fuchs wrote:
>> Hi David,
>>
>> In fact I could use 'null' on JDK 9 as well.
>> My first version of the JDK 9 test was parsing over all the .jimage
>> files under <jdk>/lib/modules - which explain why I needed to
>> use the System class loader.
>>
>> Then I switched to only parsing the bootmodules.jimage - because
>> I noticed that the results where more coherent with what I had
>> observed on 8 & 7 - but I kept using the System class loader.
>>
>> I am not sure whether we want the test for 9 should iterate over
>> the three .jimage - or continue to test only the boot .jimage.
>>
>> I have updated the JDK 9 test (refreshed the webrev in place)
>> http://cr.openjdk.java.net/~dfuchs/webrev_8066612/webrev-jdk9.00/
>> and added support for possibly running the test in the two modes
>> (I added a 'test.boot.only' system property, true by default)
>> as well as additional traces to print the loaded classes by
>> defining loader at the end (test.list.classes, false by default).
>
> A couple of initial comments:
>
>   104     static ClassLoader getClassLoaderFor(String classFileName) {
>   105         if (restrictToBoot) return null; // only bootmodules
>   106         return ClassLoaders.systemClassLoader;
>   107     }
>
> I'm not clear the intent here. If it is to return a loader for which
> loadClass could be invoked then you can always just return the system
> loader - or just Class.forName. If it is meant to the return the
> expected defining loader then it isn't doing that as the extensions
> loader is not allowed for.

The intent is to return the class loader that will be passed to
Class.forName( ). I've been fiddling & experimenting with this
test over 3 different platforms while trying to minimize the
differences - so that was my attempt at having a good place to
experiment with different strategy for loading classes.

> Similarly for:
>
>   128         static ClassLoader getFor(String classFileName) {
>   129             return systemClassLoader;
>   130         }

Oh - that's my mistake. In fact ClassLoader getClassLoaderFor(..)
was supposed to simply return ClassLoaders.getFor(...);
and all the code should be in ClassLoaders.getFor - my bad.

> Minor nit - In:
>
> 135                     System.err.println("Unexpected loader for "+c+":
> "+c.getClassLoader());
>
> c.getClassLoader() can be replaced by cl. Also put spaces around the +
> operator.

Good catch.

Thanks a lot David! Have a good night! (that's quite late - isn't it?)

-- daniel

>
> David
> (signing off for the night)
>
>> Thanks for your question, it triggered me into looking deeper
>> into what was happening :-)
>>
>> best regards,
>>
>> -- daniel
>>
>> On 04/12/14 10:05, Daniel Fuchs wrote:
>>>>> The differences between 8 & 9 are limited to:
>>>>>
>>>>>     - ClassLoader:
>>>>>          - on 8 we use 'null' (BCL)
>>>>>          - on 9 we use the system class loader.
>>>>
>>>> I haven't seen anything in JEP 220, regarding modules, that indicates
>>>> that classes currently loaded  by the boot-loader will now be loaded
>>>> by the system classloader ???
>>>
>>> In [1] towards the end:
>>>
>>> [1] http://openjdk.java.net/jeps/220
>>>
>>> "The defining class loader of the types in some existing packages
>>>   will change. Existing code that makes assumptions about the class
>>>   loaders of these types might not work correctly."
>>>   (then there is a list of specific changes).
>>>
>>> This test looks up all class names in the image files and attempt
>>> to load the corresponding class. But as indicated in [1]
>>> some of these classes are now in the Boot CL, some in the
>>> Extension CL, and some in the Application CL.
>>>
>>> So the test uses the System CL to load each class - which ensures
>>> that the loading will be delegated to the appropriate ClassLoader.
>>>
>>> best regards,
>>>
>>> -- daniel
>>