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

[David Holmes]

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.

Similarly for:

  128         static ClassLoader getFor(String classFileName) {
  129             return systemClassLoader;
  130         }

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.

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
>