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

Wed Jun 13 23:06:59 UTC 2018

On 6/13/2018 8:00 AM, Lindenmaier, Goetz wrote:

> Hi Lois, Mandy,
>
> Is there any progress on 8202605?

Hi Goetz,
I just posted an RFR for 8202605.  Would greatly appreciate your 
review.  I plan to move onto 
https://bugs.openjdk.java.net/browse/JDK-8169559 to start work on 
replacing Klass::class_loader_and_name() with hopefully a better utility 
method to use for constructing error messages that adhere to the 
recently proposed format.
Thanks,
Lois

> Feature close for jdk11 is coming up (28.6.!) and I would like
> to get this change into jdk11.  I'm now stuck with this for quite a
> while.
>
> Or should I go ahead, implement the messages as you proposed
> except for the loader names, where I could call loader_name() for
> now and you will replace it by the new method?
>
> Also, I could make a proposal for the new loader_name() in a
> change of its own, without consolidating everything?
> This would help as upcoming changes can use it right away.
>
> Best regards,
>    Goetz.
>
>> -----Original Message-----
>> From: Lois Foltan [mailto:lois.foltan at oracle.com]
>> Sent: Freitag, 8. Juni 2018 15:39
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
>> dev at openjdk.java.net runtime <hotspot-runtime-dev at openjdk.java.net>;
>> Mandy Chung <mandy.chung at oracle.com>
>> Subject: Re: RFR(M): 8199940: Print more information about class loaders in
>> IllegalAccessErrors.
>>
>>> 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()
>>>     classLoaderData::loader_name()
>>>     java_lang_ClassLoader::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.
>> Hi Goetz,
>> Glad you like the proposal.  I am currently working on consolidating all
>> the above methods you list into something more consistent for class
>> loaders.  See https://bugs.openjdk.java.net/browse/JDK-8202605.
>>
>>> 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
>>>     classLoaderData::loader_name()        --> full_loader_description
>> I will also be working on implementing new utility methods probably
>> within Klass to be used consistently by the various error messages. See
>> the RFE, https://bugs.openjdk.java.net/browse/JDK-8169559.  I may decide
>> to either remove Klass::class_loader_and_module_name() or rename it to
>> something very specific to indicate that it is the format that is used
>> for StackTraceElement toString() as documented at
>> https://docs.oracle.com/javase/9/docs/api/java/lang/StackTraceElement.ht
>> ml#toString--.
>> So there may be value in keeping it.  I will address this as well in
>> JDK-8169559.
>>
>> Thanks,
>> Lois
>>
>>> 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