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

Fri Jun 8 08:11:58 UTC 2018

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.

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