RFR(M): 8199940: Print more information about class loaders in IllegalAccessErrors.
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Mon Jun 11 15:27:52 UTC 2018
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:mandy.chung at oracle.com]
> Sent: Freitag, 8. Juni 2018 17:29
> 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,
>
> I'm glad that you like this proposal. Implementation-wise, Lois has
> the ideas to clean up the existing code and provide the utility
> functions for displaying relevant information. I agree with you that
> the existing function names are unclear and easily misunderstood.
>
> My comment inlined below.
>
> On 6/8/18 1:11 AM, Lindenmaier, Goetz wrote:
> > Hi Mandy,
> >
> > I like this proposal a lot.
> > * it contains all necessary information
> > * I like that it also contains the @id
> > * I like that the major message is in a clear sentence at the
> > beginning, the further information in parentheses at the end.
> >
> > I don't care about single or double quotes, but in the messages
> > I only saw double quotes so far. A lot of messages will have to
> > be changed to make this consistent.
> > About the implementation:
> >
> > Do I understand correctly that
> > classLoaderData::loader_name() should be changed to return either of
> > com.acme.MySameClassLoader at 423aefd2
> > 'app'
> > 'Cool lib loader' @4567 (is the space before @ intended?)
> > and Klass::class_loader_and_module_name(verbose) should return
> > test.IAE1_B in unnamed module of loader
> com.acme.MySameClassLoader at 423aefd2
> > test.IAE1_A in unnamed module of loader 'app'
> > p2.C2 in module m2x at 9.0 of loader 'app'
> > p1.C1 in module m1x at 2.0 of loader 'app'
> >
> > Also, there currently are
> > Klass::external_name()
> > Klass::class_loader_and_module_name()
>
> I think a Klass should provide a function to print `$CLASS in $MODULE of
> $LOADER` format that will be used for exception message or maybe logging
> etc.
>
> Maybe Klass::class_in_module_of_loader_name
>
> > classLoaderData::loader_name() > java_lang_ClassLoader::name()
>
> This returns the `name` field of ClassLoader instance. So this is
> loader's name.
>
> > java_lang_ClassLoader::describe_external()
>
> > These function names are often misunderstood.
> > It's unclear that classLoaderData::loader_name() and
> > java_lang_ClassLoader::name() do different things,
> > loader_name let's one assume it returns the name field
> > of the ClassLoader class.
> >
> > Klass::class_loader_and_module_name() sounds as if
> > only the name of the classloader and the name of
> > the module are printed, not the name of the class.
> >
> > Maybe we should rename like this:
> > Klass::class_loader_and_module_name() --> full_class_description
>
>
> What about Klass::class_in_module_of_loader_name
>
> > classLoaderData::loader_name() --> full_loader_description
>
> It'd be help that the name reflects what this ends up printing.
>
> I'm sure Lois will come up with better names.
>
> Mandy
>
> > Best regards,
> > Goetz.
> >
> >> -----Original Message-----
> >> From: mandy chung [mailto:mandy.chung at oracle.com]
> >> Sent: Donnerstag, 7. Juni 2018 19:49
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Lois Foltan
> >> <lois.foltan at oracle.com>
> >> Cc: hotspot-runtime-dev at openjdk.java.net
> >> Subject: Re: RFR(M): 8199940: Print more information about class loaders
> in
> >> IllegalAccessErrors.
> >>
> >> Hi Goetz,
> >>
> >> We have discussed several options to include the loader info that
> >> developers reading the exception message can easily tell the
> >> problem and reason. We look at a few exception messages and
> >> like the following most:
> >>
> >> class test.IAE1_B cannot access its superinterface test.IAE1_A
> >> (test.IAE1_B in unnamed module of loader
> com.acme.MySameClassLoader
> >> @423aefd2; test.IAE1_A in unnamed module of loader 'app')
> >>
> >> class p1.C1 cannot access private method p2.C2.method2()V
> >> (p1.C1 in module m1x at 2.0 of loader 'app'; p2.C2 in module m2x at 9.0 of
> >> loader 'app')
> >>
> >> class p.C cannot access class jdk.internal.misc.VM (p.C in unnamed
> >> module of loader 'Cool lib loader' @4567; jdk.internal.misc.VM in module
> >> java.base; java.base does not export jdk.internal.misc to the unnamed
> >> module)
> >>
> >> loader constraint violation for type pkg.Foo:
> >> class pkg.Child tried to access pkg.Parent._field1, a field of type
> >> pkg.Foo, but pkg.Child and pkg.Parent see different definitions of
> >> pkg.Foo
> >> (pkg.Child in unnamed module of loader
> pkg.ClassLoaderForChildGrandFoo
> >> @123, parent loader 'app'; pkg.Parent in unnamed module of loader
> >> pkg.ClassLoaderForParentFoo @456, parent loader 'app')
> >>
> >> Below describes the preliminary proposal of the format.
> >>
> >> If the loader name is explicitly specified then
> >> ... of loader '<name>' @<id>
> >>
> >> If the loader name is not explicitly specified
> >> ... of loader <qualified-type-name> @<id>
> >>
> >> - single-quote denotes it's explicitly specified loader name.
> >> The loader name explicitly specified may contain whitespace.
> >>
> >> - @<id> can distinct different instances with the same loader name
> >> or of the same type.
> >>
> >> - omit @<id> for built-in loaders
> >>
> >> Your feedback is appreciated.
> >>
> >> Lois is on vacation. I think it may be best for her to take this RFE
> >> and implement the new format once we agree on the proposal as this
> >> would need to update some exception wordings to make the problem
> >> and reason very clear.
> >>
> >> Mandy
> >>
> >> On 6/7/18 2:54 AM, Lindenmaier, Goetz wrote:
> >>> Hi Lois,
> >>>
> >>> any progress on this issue? I would like to still get this into jdk11
> >>> ...
> >>>
> >>> Best regards, Goetz.
> >>>
> >>>> -----Original Message----- From: Lois Foltan
> >>>> [mailto:lois.foltan at oracle.com] Sent: Freitag, 1. Juni 2018 15:26
> >>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; mandy chung
> >>>> <mandy.chung at oracle.com> Cc: hotspot-runtime-
> dev at openjdk.java.net
> >>>> Subject: Re: RFR(M): 8199940: Print more information about class
> >>>> loaders in IllegalAccessErrors.
> >>>>
> >>>> Hi Goetz,
> >>>>
> >>>> Again, thank you for making the change to the IAE error message to
> >>>> include the class loader name. Due to the recommendation of
> >>>> differing formats, we are discussing this internally to make sure
> >>>> the format we suggest is a common format for not just IAE, but for
> >>>> all error messages that may be changed in the future to include the
> >>>> class loader name. Appreciate your patience with this and hopefully
> >>>> should have an agreed upon proposal shortly.
> >>>>
> >>>> Thanks, Lois
> >>>>
> >>>> On 5/25/2018 2:39 AM, Lindenmaier, Goetz wrote:
> >>>>> Hi Mandy,
> >>>>>
> >>>>> thanks for looking at my change!
> >>>>>
> >>>>>>> I changed the code as agreed:
> >>>>>>> http://cr.openjdk.java.net/~goetz/wr18/8199940-exMsg-
> >> IllegalAccess/02/
> >>>>>>> The message is now: "class test.IAE1_B cannot access its
> >>>>>>> superinterface test.IAE1_A" if there are no names in the
> >>>>>>> loaders.
> >>>>>> For this IAE, it should include the module in the message as:
> >>>>>> class test.IAE1_B (in unnamed module @0x3d04a311) cannot access
> >>>>>> its superinterface test.IAE1_A (in module m1)
> >>>>> Wouldn't it be better if we extend class_loader_and_module_name()
> >>>>> to print module at 0x3d04a311/test.IAE1_B if we pass a flag 'verbose'
> or
> >> the
> >>>>> like? I think that is the way Lois would prefer. If the modules
> >>>>> differ, we could add some more text:
> >>>>>
> >>>>> "class module at 0x3d04a311/test.IAE1_B cannot access its
> >>>>> superinterface m1/test.IAE1_A because they are in different
> modules"
> >>>>>
> >>>>> One still would not know this was caused by using different
> >>>>> loaders in the implementation ...
> >>>>>
> >>>>>>> How should anybody know from the message that the classes
> >>>>>>> were loaded by different loaders? It gives no hint at all to
> >>>>>>> the cause of the problem.
> >>>>>> The above is one example that does not have the loader name.
> >>>>>> Each module is defined to one loader so it can derive from the
> >>>>>> module info.
> >>>>>>
> >>>>>>>> java.lang.IllegalAccessError: tried to access private
> >>>>>>>> method MySameClassLoader/m2x/p2.c2.method2()V from class
> >>>>>>>> MySameClassLoader/m1x/p1.c1
> >>>>>> What about other formats:
> >>>>>>
> >>>>>> tried to access private method p2.c2.method2()V (in module m2x)
> >>>>>> from class p1.c1 (in module m1x)
> >>>>> Hmm, as above, Lois wants me to use
> >>>>> class_loader_and_module_name(). Also, in none of my examples,
> >>>>> modules have a name or are handled by the code in any way. So
> >>>>> it's quite misleading if the modules are reported as issue while
> >>>>> it's in first place caused by the class loaders (which might
> >>>>> induce modules that then cause the error.)
> >>>>>
> >>>>>> if the loader name is highly desirable (I'm unsure yet):
> >>>>> Yes it is. It can be set by the developer and if he does so it
> >>>>> should be printed.
> >>>>> The modules here seem to be generated automatically. Maybe they
> >> should
> >>>>> derive their names from the class loaders if the relation between
> >>>>> them is that obvious?
> >>>>>
> >>>>>> tried to access private method p2.c2.method2()V (in module m2x
> >>>>>> defined to MySameClassLoader) from class p1.c1 (in module m1x
> >>>>>> defined to MySameClassLoader)
> >>>>>>
> >>>>>> Mandy
> >>>
More information about the hotspot-runtime-dev
mailing list