RFR(M): 8199852: Print more information about class loaders in LinkageErrors.

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Fri Apr 13 08:30:08 UTC 2018


Hi Mandy,

> The parent loader does not cover all cases.
> A well-defined class loader follows the parent class loader delegation model.
> But there are  custom class loaders that  can delegate to more than one class loaders.
>  ClassLoader::getParent does not show the class loader delegation it uses.
>  The class loader defining a named module may delegate to one or more class
>  loaders, each is the class loader defining one module dependence.
I know about this. Printing the parent is a first shot here.
In our internal VM (SAP JVM) we have extended ClassLoader by two arrays,
on containing strings describing the parents, and another with URL strings.
This information must be filled by the implementor of the custom class loader.
But it’s printed in all these exceptions, and helps our developers and support
teams a lot.
I might contribute this in a later change.  But as it requires changing
the class loader itself, I expect much more resistance here ��

Best regards,
  Goetz.


From: mandy chung <mandy.chung at oracle.com>
Sent: Thursday, March 29, 2018 3:33 AM
To: David Holmes <david.holmes at oracle.com>; Lois Foltan <lois.foltan at oracle.com>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
Cc: hotspot-runtime-dev at openjdk.java.net
Subject: Re: RFR(M): 8199852: Print more information about class loaders in LinkageErrors.


On 3/27/18 9:22 AM, David Holmes wrote:
Hi Lois,

<trimming>

On 27/03/2018 5:58 AM, Lois Foltan wrote:

On 3/25/2018 5:48 PM, David Holmes wrote:

Maybe it makes sense to have a set pattern for this descriptive text, and use "unnamed" if the loader has no name:

loader <name>, instanceof <class>, child of loader <name> ...

not sure how much detail about the parent is needed.

I do not support the above pattern for the descriptive text.  The supported pattern has already been documented within the API Note of StackTraceElement.toString(), https://docs.oracle.com/javase/9/docs/api/java/lang/StackTraceElement.html.

Sorry Lois but I don't see that the toString() method of StackTraceElement is an authoritive definition of how loaders should be described in other contexts.

I think Lois suggests to consider the well-defined pattern used in printing stack trace when appropriate.  It's designed to be compact and provide the adequate information for troubleshooting.

Reading JDK-8199940 and JDK-8199852, a relevant format is the exception message in IllegalAccessError (and IllegalAccessException thrown by core reflection) that I have added a comment in

class $FROM_CLASS (in $FROM_MODULE) cannot access class $TO_CLASS (in $TO_MODULE) because $TO_MODULE does not export $PACKAGE_OF_TO_CLASS to $FROM_MODULE

The format of the module is:
   if it's a named module, it will print as "module $MODULE"
   if it's an unnamed module, it will print as "unnamed module @0xnnnnnnnn" where 0xnnnnnnnn is the identity hash code of the module.

A module is defined to one class loader.

That toString definition has to combine module, loader, class and method, information in a compact form. But that is not what we are dealing with here. Had I been involved in that work I would have also argued against completely omitting loader information in the un-named case.

Omitting the builtin class loader has no loss of information for troubleshooting. The boot loader, platform loader  and app loader each has one unnamed module.   The packages defined to a named module are fixed.   It may not look obviously to which class loader/unnamed module the class of the stack frame is defined.



For example, the preference as stated for "unnamed" loaders is:

If the class loader is abuilt-in class loader <https://docs.oracle.com/javase/9/docs/api/java/lang/ClassLoader.html#builtinLoaders><https://docs.oracle.com/javase/9/docs/api/java/lang/ClassLoader.html#builtinLoaders>or is not named then the first element and its following|"/"|are omitted as shown in "|acme at 2.1/org.acme.Lib.test(Lib.java:80)|"<mailto:|acme at 2.1/org.acme.Lib.test(Lib.java:80)|>

Again, Klass::class_loader_module_name() already implement and adhere to this pattern.

Yes but we are not dealing with module names here. And the fact it completely ignores un-named loaders makes it inherently unsuitable for the current context.

The issue of whether information about the parent loader is suitable for the error message or should be left for logging, depends on how you expect these messages to help. If the idea is for the exception message to provide enough information for a support engineer to be able to diagnose a problem without asking the submitter to re-run with logging enabled, then you want to gather as much relevant information as possible. But a line has to be drawn somewhere. I don't think listing the loader's parent is excessive in this case, but nor do I see it as essential.

I share Lois's concern in including the parent loader in the error message.

The parent loader does not cover all cases.   A well-defined class loader follows the parent class loader delegation model.  But there are  custom class loaders that can delegate to more than one class loaders.  ClassLoader::getParent does not show the class loader delegation it uses.  The class loader defining a named module may delegate to one or more class loaders, each is the class loader defining one module dependence.  If M requires M1 and M2, M1 and M2 may be defined to different class loaders while M's loader::getParent may return a class loader that is not M1's and M2's loader.

In any case, improving the error message for LinkageError is good but it probably worths taking the time to agree on the format for a named and unnamed class loader and its type.  I will give some thought and add the JBS issue of any suggestion.

Mandy


More information about the hotspot-runtime-dev mailing list