RFR(M): 8199940: Print more information about class loaders in IllegalAccessErrors.

mandy chung mandy.chung at oracle.com
Fri Jun 8 15:28:34 UTC 2018


Hi Goetz,

I'm glad that you like this proposal.  Implementation-wise, Lois has
the ideas to clean up the existing code and provide the utility
functions for displaying relevant information.  I agree with you that
the existing function names are unclear and easily misunderstood.

My comment inlined below.

On 6/8/18 1:11 AM, Lindenmaier, Goetz wrote:
> Hi Mandy,
> 
> I like this proposal a lot.
>   * it contains all necessary information
>   * I like that it also contains the @id
>   * I like that the major message is in a clear sentence at the
>     beginning, the further information in parentheses at the end.
> 
> I don't care about single or double quotes, but in the messages
> I only saw double quotes so far. A lot of messages will have to
> be changed to make this consistent.
> About the implementation:
> 
> Do I understand correctly that
> classLoaderData::loader_name() should be changed to return either of
>    com.acme.MySameClassLoader at 423aefd2
>    'app'
>    'Cool lib loader' @4567    (is the space before @ intended?)
> and Klass::class_loader_and_module_name(verbose) should return
>    test.IAE1_B in unnamed module of loader com.acme.MySameClassLoader at 423aefd2
>    test.IAE1_A in unnamed module of loader 'app'
>    p2.C2 in module m2x at 9.0 of loader 'app'
>    p1.C1 in module m1x at 2.0 of loader 'app'
> 
> Also, there currently are
>    Klass::external_name()
>    Klass::class_loader_and_module_name()

I think a Klass should provide a function to print `$CLASS in $MODULE of 
$LOADER` format that will be used for exception message or maybe logging 
etc.

Maybe Klass::class_in_module_of_loader_name

>    classLoaderData::loader_name() >    java_lang_ClassLoader::name()

This returns the `name` field of ClassLoader instance.  So this is 
loader's name.

>    java_lang_ClassLoader::describe_external()

> These function names are often misunderstood.
> It's unclear that classLoaderData::loader_name() and
> java_lang_ClassLoader::name() do different things,
> loader_name let's one assume it returns the name field
> of the ClassLoader class.
> 
> Klass::class_loader_and_module_name() sounds as if
> only the name of the classloader and the name of
> the module are printed, not the name of the class.
> 
> Maybe we should rename like this:
>    Klass::class_loader_and_module_name() --> full_class_description


What about Klass::class_in_module_of_loader_name

>    classLoaderData::loader_name()        --> full_loader_description

It'd be help that the name reflects what this ends up printing.

I'm sure Lois will come up with better names.

Mandy

> Best regards,
>    Goetz.
> 
>> -----Original Message-----
>> From: mandy chung [mailto:mandy.chung at oracle.com]
>> Sent: Donnerstag, 7. Juni 2018 19:49
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Lois Foltan
>> <lois.foltan at oracle.com>
>> Cc: hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR(M): 8199940: Print more information about class loaders in
>> IllegalAccessErrors.
>>
>> Hi Goetz,
>>
>> We have discussed several options to include the loader info that
>> developers reading the exception message can easily tell the
>> problem and reason. We look at a few exception messages and
>> like the following most:
>>
>> class test.IAE1_B cannot access its superinterface test.IAE1_A
>> (test.IAE1_B in unnamed module of loader com.acme.MySameClassLoader
>> @423aefd2; test.IAE1_A in unnamed module of loader 'app')
>>
>> class p1.C1 cannot access private method p2.C2.method2()V
>> (p1.C1 in module m1x at 2.0 of loader 'app'; p2.C2 in module m2x at 9.0 of
>> loader 'app')
>>
>> class p.C cannot access class jdk.internal.misc.VM (p.C in unnamed
>> module of loader 'Cool lib loader' @4567; jdk.internal.misc.VM in module
>> java.base; java.base does not export jdk.internal.misc to the unnamed
>> module)
>>
>> loader constraint violation for type pkg.Foo:
>> class pkg.Child tried to access pkg.Parent._field1, a field of type
>> pkg.Foo, but pkg.Child and pkg.Parent see different definitions of
>> pkg.Foo
>> (pkg.Child in unnamed module of loader pkg.ClassLoaderForChildGrandFoo
>> @123, parent loader 'app'; pkg.Parent in unnamed module of loader
>> pkg.ClassLoaderForParentFoo @456, parent loader 'app')
>>
>> Below describes the preliminary proposal of the format.
>>
>> If the loader name is explicitly specified then
>>     ... of loader '<name>' @<id>
>>
>> If the loader name is not explicitly specified
>>     ... of loader <qualified-type-name> @<id>
>>
>> - single-quote denotes it's explicitly specified loader name.
>>     The loader name explicitly specified may contain whitespace.
>>
>> - @<id> can distinct different instances with the same loader name
>>     or of the same type.
>>
>> - omit @<id> for built-in loaders
>>
>> Your feedback is appreciated.
>>
>> Lois is on vacation.  I think it may be best for her to take this RFE
>> and implement the new format once we agree on the proposal as this
>> would need to update some exception wordings to make the problem
>> and reason very clear.
>>
>> Mandy
>>
>> On 6/7/18 2:54 AM, Lindenmaier, Goetz wrote:
>>> Hi Lois,
>>>
>>> any progress on this issue? I would like to still get this into jdk11
>>> ...
>>>
>>> Best regards, Goetz.
>>>
>>>> -----Original Message----- From: Lois Foltan
>>>> [mailto:lois.foltan at oracle.com] Sent: Freitag, 1. Juni 2018 15:26
>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; mandy chung
>>>> <mandy.chung at oracle.com> Cc: hotspot-runtime-dev at openjdk.java.net
>>>> Subject: Re: RFR(M): 8199940: Print more information about class
>>>> loaders in IllegalAccessErrors.
>>>>
>>>> Hi Goetz,
>>>>
>>>> Again, thank you for making the change to the IAE error message to
>>>> include the class loader name.  Due to the recommendation of
>>>> differing formats, we are discussing this internally to make sure
>>>> the format we suggest is a common format for not just IAE, but for
>>>> all error messages that may be changed in the future to include the
>>>> class loader name. Appreciate your patience with this and hopefully
>>>> should have an agreed upon proposal shortly.
>>>>
>>>> Thanks, Lois
>>>>
>>>> On 5/25/2018 2:39 AM, Lindenmaier, Goetz wrote:
>>>>> Hi Mandy,
>>>>>
>>>>> thanks for looking at my change!
>>>>>
>>>>>>> I changed  the code as agreed:
>>>>>>> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-
>> IllegalAccess/02/
>>>>>>> The message is now: "class test.IAE1_B cannot access its
>>>>>>> superinterface test.IAE1_A" if there are no names in the
>>>>>>> loaders.
>>>>>> For this IAE, it should include the module in the message as:
>>>>>> class test.IAE1_B (in unnamed module @0x3d04a311) cannot access
>>>>>> its superinterface test.IAE1_A (in module m1)
>>>>> Wouldn't it be better if we extend class_loader_and_module_name()
>>>>> to print module at 0x3d04a311/test.IAE1_B if we pass a flag 'verbose' or
>> the
>>>>> like? I think that is the  way Lois would prefer. If the modules
>>>>> differ, we could add some more text:
>>>>>
>>>>> "class module at 0x3d04a311/test.IAE1_B cannot access its
>>>>> superinterface m1/test.IAE1_A because they are in different modules"
>>>>>
>>>>> One still would not know this was caused by using different
>>>>> loaders in the implementation ...
>>>>>
>>>>>>> How should anybody know from the message that the classes
>>>>>>> were loaded by different loaders? It gives no hint at all to
>>>>>>> the cause of the problem.
>>>>>> The above is one example that does not have the loader name.
>>>>>> Each module is defined to one loader so it can derive from the
>>>>>> module info.
>>>>>>
>>>>>>>> java.lang.IllegalAccessError: tried to access private
>>>>>>>> method MySameClassLoader/m2x/p2.c2.method2()V from class
>>>>>>>> MySameClassLoader/m1x/p1.c1
>>>>>> What about other formats:
>>>>>>
>>>>>> tried to access private method p2.c2.method2()V (in module m2x)
>>>>>> from class p1.c1 (in module m1x)
>>>>> Hmm, as above, Lois wants me to use
>>>>> class_loader_and_module_name(). Also, in none of my examples,
>>>>> modules have a name or are handled by the code in any way.  So
>>>>> it's quite misleading if the modules are reported as issue while
>>>>> it's in first place caused by the class loaders (which might
>>>>> induce modules that then cause the error.)
>>>>>
>>>>>> if the loader name is highly desirable (I'm unsure yet):
>>>>> Yes it is. It can be set by the developer and if he does so it
>>>>> should be printed.
>>>>> The modules here seem to be generated automatically.  Maybe they
>> should
>>>>> derive their names from the class loaders if the relation between
>>>>> them is that obvious?
>>>>>
>>>>>> tried to access private method p2.c2.method2()V (in module m2x
>>>>>> defined to MySameClassLoader) from class p1.c1 (in module m1x
>>>>>> defined to MySameClassLoader)
>>>>>>
>>>>>> Mandy
>>>


More information about the hotspot-runtime-dev mailing list