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

classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
Hi,

I finally prepared the webrev for this change.
I simplified the messages wrt. to my first intent that was found too
verbose.

The IllegalAccessErrors in classFileParser report more verbose
information about the class loaders.
As I understand, the class loaders are the reason for the error here,
so I think information about them is useful. See also the tests 1-3.
I added reporting whether the class is abstract.

In linkResolver, I just switch to class_loader_and_module_name()
to report a more verbose class name as Lois requested.
I removed mentioning the resolved class in the method case.
I report the modifiers of methods/fields.
But as I understand, even here the class loader can be the reason
of the Error, see tests 6-8.

Please review.  I'm happy to improve the messages further :)
http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-IllegalAccess/01/

Best regards,
  Goetz.
Reply | Threaded
Open this post in threaded view
|

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

Lois Foltan
On 5/16/2018 9:30 AM, Lindenmaier, Goetz wrote:

> Hi,
>
> I finally prepared the webrev for this change.
> I simplified the messages wrt. to my first intent that was found too
> verbose.
>
> The IllegalAccessErrors in classFileParser report more verbose
> information about the class loaders.
> As I understand, the class loaders are the reason for the error here,
> so I think information about them is useful. See also the tests 1-3.
> I added reporting whether the class is abstract.
>
> In linkResolver, I just switch to class_loader_and_module_name()
> to report a more verbose class name as Lois requested.
> I removed mentioning the resolved class in the method case.
> I report the modifiers of methods/fields.
> But as I understand, even here the class loader can be the reason
> of the Error, see tests 6-8.
>
> Please review.  I'm happy to improve the messages further :)
> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-IllegalAccess/01/
>
> Best regards,
>    Goetz.
Hi Goetz,

Thanks for making this change, improving IAEs to include class loader
names has been on our radar as well.  Here are my review comments to
hopefully move this change forward.

- src/hotspot/share/classfile/classFileParser.cpp
Based on further discussion in
https://bugs.openjdk.java.net/browse/JDK-8199940, I believe you
suggested changing the calls from describe_external() to loader_name(). 
I would support that change or even a change to use
class_loader_and_module_name().

- src/hotspot/share/interpreter/linkResolver.cpp
I think all the changes in this file look great!  I did write a new test
to demonstrate how an IAE looks when a type with correct module
readability and package exportability tries to access a private method
within another type. The IAE generated will contain the following:

java.lang.IllegalAccessError: tried to access private method
MySameClassLoader/m2x/p2.c2.method2()V from class
MySameClassLoader/m1x/p1.c1

Once your change is in, I will follow up with an RFE to include this
test with the other accessibility tests in
open/test/hotspot/jtreg/runtime/modules/AccessCheck.

Thanks,
Lois






Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
Hi Lois,

I changed  the code as agreed:
http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-IllegalAccess/02/

I'm not really sure this is good:

 interface IAE1_A {
     public IAE1_D gen();
 }
 class IAE1_B implements IAE1_A {
     public IAE1_D gen() {
         return null;
     }
 }
A is loaded by "app", B by the custom loader.

The message is now:
"class test.IAE1_B cannot access its superinterface test.IAE1_A"
if there are no names in the loaders.

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.

Or do I misunderstand the test?

Best regards,
  Goetz.



> -----Original Message-----
> From: Lois Foltan <[hidden email]>
> Sent: Thursday, May 24, 2018 10:12 PM
> To: Lindenmaier, Goetz <[hidden email]>; hotspot-runtime-
> [hidden email]
> Subject: Re: RFR(M): 8199940: Print more information about class loaders in
> IllegalAccessErrors.
>
> On 5/16/2018 9:30 AM, Lindenmaier, Goetz wrote:
>
> > Hi,
> >
> > I finally prepared the webrev for this change.
> > I simplified the messages wrt. to my first intent that was found too
> > verbose.
> >
> > The IllegalAccessErrors in classFileParser report more verbose
> > information about the class loaders.
> > As I understand, the class loaders are the reason for the error here,
> > so I think information about them is useful. See also the tests 1-3.
> > I added reporting whether the class is abstract.
> >
> > In linkResolver, I just switch to class_loader_and_module_name()
> > to report a more verbose class name as Lois requested.
> > I removed mentioning the resolved class in the method case.
> > I report the modifiers of methods/fields.
> > But as I understand, even here the class loader can be the reason
> > of the Error, see tests 6-8.
> >
> > Please review.  I'm happy to improve the messages further :)
> > http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-IllegalAccess/01/
> >
> > Best regards,
> >    Goetz.
> Hi Goetz,
>
> Thanks for making this change, improving IAEs to include class loader
> names has been on our radar as well.  Here are my review comments to
> hopefully move this change forward.
>
> - src/hotspot/share/classfile/classFileParser.cpp
> Based on further discussion in
> https://bugs.openjdk.java.net/browse/JDK-8199940, I believe you
> suggested changing the calls from describe_external() to loader_name().
> I would support that change or even a change to use
> class_loader_and_module_name().
>
> - src/hotspot/share/interpreter/linkResolver.cpp
> I think all the changes in this file look great!  I did write a new test
> to demonstrate how an IAE looks when a type with correct module
> readability and package exportability tries to access a private method
> within another type. The IAE generated will contain the following:
>
> java.lang.IllegalAccessError: tried to access private method
> MySameClassLoader/m2x/p2.c2.method2()V from class
> MySameClassLoader/m1x/p1.c1
>
> Once your change is in, I will follow up with an RFE to include this
> test with the other accessibility tests in
> open/test/hotspot/jtreg/runtime/modules/AccessCheck.
>
> Thanks,
> Lois
>
>
>
>
>

Reply | Threaded
Open this post in threaded view
|

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

Mandy Chung


On 5/24/18 2:12 PM, Lindenmaier, Goetz wrote:

> Hi Lois,
>
> I changed  the code as agreed:
> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-IllegalAccess/02/
>
> I'm not really sure this is good:
>
>   interface IAE1_A {
>       public IAE1_D gen();
>   }
>   class IAE1_B implements IAE1_A {
>       public IAE1_D gen() {
>           return null;
>       }
>   }
> A is loaded by "app", B by the custom loader.
>
> 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)

> 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)

if the loader name is highly desirable (I'm unsure yet):

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
Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
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@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@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
Reply | Threaded
Open this post in threaded view
|

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

Lois Foltan
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@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@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

Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
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:[hidden email]]
> Sent: Freitag, 1. Juni 2018 15:26
> To: Lindenmaier, Goetz <[hidden email]>; mandy chung
> <[hidden email]>
> Cc: [hidden email]
> 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@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@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

Reply | Threaded
Open this post in threaded view
|

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

Lois Foltan
On 6/7/2018 5:54 AM, Lindenmaier, Goetz wrote:

> Hi Lois,
>
> any progress on this issue? I would like to still get this into jdk11 ...
Hi Goetz,

Yes, Mandy & I have made good progress on formulating a preliminary
proposal consulting with Alex Buckley on suggested wording.  She will be
sending that to you shortly.

Thanks,
Lois

>
> Best regards,
>    Goetz.
>
>> -----Original Message-----
>> From: Lois Foltan [mailto:[hidden email]]
>> Sent: Freitag, 1. Juni 2018 15:26
>> To: Lindenmaier, Goetz <[hidden email]>; mandy chung
>> <[hidden email]>
>> Cc: [hidden email]
>> 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@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@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

Reply | Threaded
Open this post in threaded view
|

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

Mandy Chung
In reply to this post by Lindenmaier, Goetz
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@2.0 of loader 'app'; p2.C2 in module m2x@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:[hidden email]] Sent: Freitag, 1. Juni 2018 15:26
>> To: Lindenmaier, Goetz <[hidden email]>; mandy chung
>> <[hidden email]> Cc: [hidden email]
>> 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@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@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
>
Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
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@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@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.

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

Best regards,
  Goetz.

> -----Original Message-----
> From: mandy chung [mailto:[hidden email]]
> Sent: Donnerstag, 7. Juni 2018 19:49
> To: Lindenmaier, Goetz <[hidden email]>; Lois Foltan
> <[hidden email]>
> Cc: [hidden email]
> 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@2.0 of loader 'app'; p2.C2 in module m2x@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:[hidden email]] Sent: Freitag, 1. Juni 2018 15:26
> >> To: Lindenmaier, Goetz <[hidden email]>; mandy chung
> >> <[hidden email]> Cc: [hidden email]
> >> 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@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@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
> >
Reply | Threaded
Open this post in threaded view
|

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

Lois Foltan
In reply to this post by Lindenmaier, Goetz
> 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@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@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.html#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:[hidden email]]
>> Sent: Donnerstag, 7. Juni 2018 19:49
>> To: Lindenmaier, Goetz <[hidden email]>; Lois Foltan
>> <[hidden email]>
>> Cc: [hidden email]
>> 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@2.0 of loader 'app'; p2.C2 in module m2x@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
Reply | Threaded
Open this post in threaded view
|

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

Mandy Chung
In reply to this post by Lindenmaier, Goetz
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@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@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:[hidden email]]
>> Sent: Donnerstag, 7. Juni 2018 19:49
>> To: Lindenmaier, Goetz <[hidden email]>; Lois Foltan
>> <[hidden email]>
>> Cc: [hidden email]
>> 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@2.0 of loader 'app'; p2.C2 in module m2x@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:[hidden email]] Sent: Freitag, 1. Juni 2018 15:26
>>>> To: Lindenmaier, Goetz <[hidden email]>; mandy chung
>>>> <[hidden email]> Cc: [hidden email]
>>>> 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@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@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
>>>
Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
Hi Mandy,

I will comment all the details once there is a RFR, which I hope comes soon :)
Just one point:

> >    classLoaderData::loader_name() >    java_lang_ClassLoader::name()
> This returns the `name` field of ClassLoader instance.  So this is
> loader's name.
This is not true. It prints the class name with '/' in most cases.
   InstanceKlass::cast((loader)->klass())->name()->as_C_string());
Except during unloading.

Best regards,
  Goetz.



> -----Original Message-----
> From: mandy chung [mailto:[hidden email]]
> Sent: Freitag, 8. Juni 2018 17:29
> To: Lindenmaier, Goetz <[hidden email]>; Lois Foltan
> <[hidden email]>
> Cc: [hidden email]
> Subject: Re: RFR(M): 8199940: Print more information about class loaders in
> IllegalAccessErrors.
>
> 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@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@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:[hidden email]]
> >> Sent: Donnerstag, 7. Juni 2018 19:49
> >> To: Lindenmaier, Goetz <[hidden email]>; Lois Foltan
> >> <[hidden email]>
> >> Cc: [hidden email]
> >> 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@2.0 of loader 'app'; p2.C2 in module m2x@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:[hidden email]] Sent: Freitag, 1. Juni 2018 15:26
> >>>> To: Lindenmaier, Goetz <[hidden email]>; mandy chung
> >>>> <[hidden email]> Cc: hotspot-runtime-
> [hidden email]
> >>>> 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@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@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
> >>>
Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
In reply to this post by Lois Foltan
Hi Lois, Mandy,

Is there any progress on 8202605?
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:[hidden email]]
> Sent: Freitag, 8. Juni 2018 15:39
> To: Lindenmaier, Goetz <[hidden email]>; hotspot-runtime-
> [hidden email] runtime <[hidden email]>;
> Mandy Chung <[hidden email]>
> 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@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@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:[hidden email]]
> >> Sent: Donnerstag, 7. Juni 2018 19:49
> >> To: Lindenmaier, Goetz <[hidden email]>; Lois Foltan
> >> <[hidden email]>
> >> Cc: [hidden email]
> >> 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@2.0 of loader 'app'; p2.C2 in module m2x@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
Reply | Threaded
Open this post in threaded view
|

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

Lois Foltan
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:[hidden email]]
>> Sent: Freitag, 8. Juni 2018 15:39
>> To: Lindenmaier, Goetz <[hidden email]>; hotspot-runtime-
>> [hidden email] runtime <[hidden email]>;
>> Mandy Chung <[hidden email]>
>> 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@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@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:[hidden email]]
>>>> Sent: Donnerstag, 7. Juni 2018 19:49
>>>> To: Lindenmaier, Goetz <[hidden email]>; Lois Foltan
>>>> <[hidden email]>
>>>> Cc: [hidden email]
>>>> 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@2.0 of loader 'app'; p2.C2 in module m2x@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

Reply | Threaded
Open this post in threaded view
|

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

Mandy Chung
In reply to this post by Mandy Chung
Hi Goetz,

Lois, Harold, and I discussed briefly how to help this patch move
forward independent of JDK-8169559 (that intends to replace
Klass::class_loader_and_module_name with a better utility method).

The format we propose is to change from:
   $C1 cannot access its superinterface $C2

TO:
   $C1 cannot access its superinterface $C2 ($C1 is in $M1 of loader
$L1; $C2 is in $M2 of loader $L2)

The above will depend on JDK-8169559.

The intermediate step is for JDK-8199940 to append the loader info in IAE:
  $C1 cannot access its superinterface $C2 ($C1 is in loader $L1; $C2 is
in loader $L2)

This will depend on the new loader_name_and_id function introduced by
JDK-8202605. JDK-8169559 can add the module info in the exception
messages as a follow-up issue (these IAEs don't have any module info in
the current implementation anyway.)

For IAE in member access, I suggest to rephrase the message from:

tried to access method test.IllegalAccessErrorTest.iae4_m()V from class
test.Runner4

TO:

class test.Runner4 tried to access private method
test.IllegalAccessErrorTest.iae4_m()V (test.Runner4  is in loader 'app';
test.IllegalAccessErrorTest. is in loader 'app')

What do you think?
Mandy

On 5/24/18 3:15 PM, mandy chung wrote:
>
>
> On 5/24/18 2:12 PM, Lindenmaier, Goetz wrote:
>> Hi Lois,
>>
>> I changed  the code as agreed:
>> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-IllegalAccess/02/
:

> 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)
>
> 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)
>
> if the loader name is highly desirable (I'm unsure yet):
>
> 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
Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
Hi Mandy,

that sounds good!

It makes sure my part still reaches jdk 11. Adding the module info
later is a useful further improvement then.

I'll post a new webrev once Lois' change JDK-8202605 pops up
in the repo.

Best regards,
  Goetz.

> -----Original Message-----
> From: mandy chung [mailto:[hidden email]]
> Sent: Montag, 18. Juni 2018 21:24
> To: Lindenmaier, Goetz <[hidden email]>; 'Lois Foltan'
> <[hidden email]>
> Cc: [hidden email]
> Subject: Re: RFR(M): 8199940: Print more information about class loaders in
> IllegalAccessErrors.
>
> Hi Goetz,
>
> Lois, Harold, and I discussed briefly how to help this patch move
> forward independent of JDK-8169559 (that intends to replace
> Klass::class_loader_and_module_name with a better utility method).
>
> The format we propose is to change from:
>    $C1 cannot access its superinterface $C2
>
> TO:
>    $C1 cannot access its superinterface $C2 ($C1 is in $M1 of loader
> $L1; $C2 is in $M2 of loader $L2)
>
> The above will depend on JDK-8169559.
>
> The intermediate step is for JDK-8199940 to append the loader info in IAE:
>   $C1 cannot access its superinterface $C2 ($C1 is in loader $L1; $C2 is
> in loader $L2)
>
> This will depend on the new loader_name_and_id function introduced by
> JDK-8202605. JDK-8169559 can add the module info in the exception
> messages as a follow-up issue (these IAEs don't have any module info in
> the current implementation anyway.)
>
> For IAE in member access, I suggest to rephrase the message from:
>
> tried to access method test.IllegalAccessErrorTest.iae4_m()V from class
> test.Runner4
>
> TO:
>
> class test.Runner4 tried to access private method
> test.IllegalAccessErrorTest.iae4_m()V (test.Runner4  is in loader 'app';
> test.IllegalAccessErrorTest. is in loader 'app')
>
> What do you think?
> Mandy
>
> On 5/24/18 3:15 PM, mandy chung wrote:
> >
> >
> > On 5/24/18 2:12 PM, Lindenmaier, Goetz wrote:
> >> Hi Lois,
> >>
> >> I changed  the code as agreed:
> >> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-
> IllegalAccess/02/
> :
>
> > 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)
> >
> > 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)
> >
> > if the loader name is highly desirable (I'm unsure yet):
> >
> > 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
Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
In reply to this post by Mandy Chung
Hi,

I implemented what Mandy suggested:
http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-IllegalAccess/03/

For this format, I could imagine following simplification of the expression in brackets:

If both loaders are the same
    If loader1 is well known
        print nothing
   else
        print "(in loader test.IAE_Loader1 @id)"
  fi
else
   print  "(test.IAE1_B is in loader test.IAE_Loader1 @id; test.IAE1_A is in loader 'app')"
fi

... I would need to improve my tests ...
Maybe a follow up after JDK-8169559?

Best regards,
  Goetz.



> -----Original Message-----
> From: mandy chung [mailto:[hidden email]]
> Sent: Montag, 18. Juni 2018 21:24
> To: Lindenmaier, Goetz <[hidden email]>; 'Lois Foltan'
> <[hidden email]>
> Cc: [hidden email]
> Subject: Re: RFR(M): 8199940: Print more information about class loaders in
> IllegalAccessErrors.
>
> Hi Goetz,
>
> Lois, Harold, and I discussed briefly how to help this patch move
> forward independent of JDK-8169559 (that intends to replace
> Klass::class_loader_and_module_name with a better utility method).
>
> The format we propose is to change from:
>    $C1 cannot access its superinterface $C2
>
> TO:
>    $C1 cannot access its superinterface $C2 ($C1 is in $M1 of loader
> $L1; $C2 is in $M2 of loader $L2)
>
> The above will depend on JDK-8169559.
>
> The intermediate step is for JDK-8199940 to append the loader info in IAE:
>   $C1 cannot access its superinterface $C2 ($C1 is in loader $L1; $C2 is
> in loader $L2)
>
> This will depend on the new loader_name_and_id function introduced by
> JDK-8202605. JDK-8169559 can add the module info in the exception
> messages as a follow-up issue (these IAEs don't have any module info in
> the current implementation anyway.)
>
> For IAE in member access, I suggest to rephrase the message from:
>
> tried to access method test.IllegalAccessErrorTest.iae4_m()V from class
> test.Runner4
>
> TO:
>
> class test.Runner4 tried to access private method
> test.IllegalAccessErrorTest.iae4_m()V (test.Runner4  is in loader 'app';
> test.IllegalAccessErrorTest. is in loader 'app')
>
> What do you think?
> Mandy
>
> On 5/24/18 3:15 PM, mandy chung wrote:
> >
> >
> > On 5/24/18 2:12 PM, Lindenmaier, Goetz wrote:
> >> Hi Lois,
> >>
> >> I changed  the code as agreed:
> >> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-
> IllegalAccess/02/
> :
>
> > 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)
> >
> > 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)
> >
> > if the loader name is highly desirable (I'm unsure yet):
> >
> > 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
Reply | Threaded
Open this post in threaded view
|

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

Mandy Chung


On 6/19/18 6:58 AM, Lindenmaier, Goetz wrote:

> Hi,
>
> I implemented what Mandy suggested:
> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-IllegalAccess/03/
>
> For this format, I could imagine following simplification of the expression in brackets:
>
> If both loaders are the same
>      If loader1 is well known
>          print nothing

What is a well known loader?  Are you referring to the built-in loaders?

Mandy

>     else
>          print "(in loader test.IAE_Loader1 @id)"
>    fi
> else
>     print  "(test.IAE1_B is in loader test.IAE_Loader1 @id; test.IAE1_A is in loader 'app')"
> fi
>
> ... I would need to improve my tests ...
> Maybe a follow up after JDK-8169559?
>
> Best regards,
>    Goetz.
>
>
>
>> -----Original Message-----
>> From: mandy chung [mailto:[hidden email]]
>> Sent: Montag, 18. Juni 2018 21:24
>> To: Lindenmaier, Goetz <[hidden email]>; 'Lois Foltan'
>> <[hidden email]>
>> Cc: [hidden email]
>> Subject: Re: RFR(M): 8199940: Print more information about class loaders in
>> IllegalAccessErrors.
>>
>> Hi Goetz,
>>
>> Lois, Harold, and I discussed briefly how to help this patch move
>> forward independent of JDK-8169559 (that intends to replace
>> Klass::class_loader_and_module_name with a better utility method).
>>
>> The format we propose is to change from:
>>     $C1 cannot access its superinterface $C2
>>
>> TO:
>>     $C1 cannot access its superinterface $C2 ($C1 is in $M1 of loader
>> $L1; $C2 is in $M2 of loader $L2)
>>
>> The above will depend on JDK-8169559.
>>
>> The intermediate step is for JDK-8199940 to append the loader info in IAE:
>>    $C1 cannot access its superinterface $C2 ($C1 is in loader $L1; $C2 is
>> in loader $L2)
>>
>> This will depend on the new loader_name_and_id function introduced by
>> JDK-8202605. JDK-8169559 can add the module info in the exception
>> messages as a follow-up issue (these IAEs don't have any module info in
>> the current implementation anyway.)
>>
>> For IAE in member access, I suggest to rephrase the message from:
>>
>> tried to access method test.IllegalAccessErrorTest.iae4_m()V from class
>> test.Runner4
>>
>> TO:
>>
>> class test.Runner4 tried to access private method
>> test.IllegalAccessErrorTest.iae4_m()V (test.Runner4  is in loader 'app';
>> test.IllegalAccessErrorTest. is in loader 'app')
>>
>> What do you think?
>> Mandy
>>
>> On 5/24/18 3:15 PM, mandy chung wrote:
>>>
>>>
>>> On 5/24/18 2:12 PM, Lindenmaier, Goetz wrote:
>>>> Hi Lois,
>>>>
>>>> I changed  the code as agreed:
>>>> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-
>> IllegalAccess/02/
>> :
>>
>>> 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)
>>>
>>> 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)
>>>
>>> if the loader name is highly desirable (I'm unsure yet):
>>>
>>> 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
Reply | Threaded
Open this post in threaded view
|

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

Lindenmaier, Goetz
Hi Mandy,

> >      If loader1 is well known
> >          print nothing
> What is a well known loader?  Are you referring to the built-in loaders?
Yes, I would propose to check cld->is_system_class_loader_data() || cld->is_platform_class_loader_data()
as in other places.

Best regards,
  Goetz



> -----Original Message-----
> From: mandy chung <[hidden email]>
> Sent: Tuesday, June 19, 2018 5:03 PM
> To: Lindenmaier, Goetz <[hidden email]>; 'Lois Foltan'
> <[hidden email]>
> Cc: [hidden email]
> Subject: Re: RFR(M): 8199940: Print more information about class loaders in
> IllegalAccessErrors.
>
>
>
> On 6/19/18 6:58 AM, Lindenmaier, Goetz wrote:
> > Hi,
> >
> > I implemented what Mandy suggested:
> > http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-IllegalAccess/03/
> >
> > For this format, I could imagine following simplification of the expression in
> brackets:
> >
> > If both loaders are the same
> >      If loader1 is well known
> >          print nothing
>
> What is a well known loader?  Are you referring to the built-in loaders?
>
> Mandy
>
> >     else
> >          print "(in loader test.IAE_Loader1 @id)"
> >    fi
> > else
> >     print  "(test.IAE1_B is in loader test.IAE_Loader1 @id; test.IAE1_A is in
> loader 'app')"
> > fi
> >
> > ... I would need to improve my tests ...
> > Maybe a follow up after JDK-8169559?
> >
> > Best regards,
> >    Goetz.
> >
> >
> >
> >> -----Original Message-----
> >> From: mandy chung [mailto:[hidden email]]
> >> Sent: Montag, 18. Juni 2018 21:24
> >> To: Lindenmaier, Goetz <[hidden email]>; 'Lois Foltan'
> >> <[hidden email]>
> >> Cc: [hidden email]
> >> Subject: Re: RFR(M): 8199940: Print more information about class loaders
> in
> >> IllegalAccessErrors.
> >>
> >> Hi Goetz,
> >>
> >> Lois, Harold, and I discussed briefly how to help this patch move
> >> forward independent of JDK-8169559 (that intends to replace
> >> Klass::class_loader_and_module_name with a better utility method).
> >>
> >> The format we propose is to change from:
> >>     $C1 cannot access its superinterface $C2
> >>
> >> TO:
> >>     $C1 cannot access its superinterface $C2 ($C1 is in $M1 of loader
> >> $L1; $C2 is in $M2 of loader $L2)
> >>
> >> The above will depend on JDK-8169559.
> >>
> >> The intermediate step is for JDK-8199940 to append the loader info in IAE:
> >>    $C1 cannot access its superinterface $C2 ($C1 is in loader $L1; $C2 is
> >> in loader $L2)
> >>
> >> This will depend on the new loader_name_and_id function introduced by
> >> JDK-8202605. JDK-8169559 can add the module info in the exception
> >> messages as a follow-up issue (these IAEs don't have any module info in
> >> the current implementation anyway.)
> >>
> >> For IAE in member access, I suggest to rephrase the message from:
> >>
> >> tried to access method test.IllegalAccessErrorTest.iae4_m()V from class
> >> test.Runner4
> >>
> >> TO:
> >>
> >> class test.Runner4 tried to access private method
> >> test.IllegalAccessErrorTest.iae4_m()V (test.Runner4  is in loader 'app';
> >> test.IllegalAccessErrorTest. is in loader 'app')
> >>
> >> What do you think?
> >> Mandy
> >>
> >> On 5/24/18 3:15 PM, mandy chung wrote:
> >>>
> >>>
> >>> On 5/24/18 2:12 PM, Lindenmaier, Goetz wrote:
> >>>> Hi Lois,
> >>>>
> >>>> I changed  the code as agreed:
> >>>> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-
> >> IllegalAccess/02/
> >> :
> >>
> >>> 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)
> >>>
> >>> 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)
> >>>
> >>> if the loader name is highly desirable (I'm unsure yet):
> >>>
> >>> 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
12