Request Review: JDK-6479237 (cl) Add support for classloader names

Mandy Chung mandy.chung at
Tue Nov 1 23:42:18 UTC 2016

Hi Daniel,

Here is the updated webrev incorporating your feedback:

> On Nov 1, 2016, at 7:04 AM, Daniel Fuchs <daniel.fuchs at> wrote:
> :
> 334             s += declaringClass;
> but should line 334 instead be
>   s = (s.isEmpty() ? declaringClass : s + "/" + declaringClass;
> Otherwise "/" will be missing after module at version..

Good catch.  I added a test for this.

> Also should there be some asserts somewhere verifying that moduleVersion
> is null or empty when moduleName is null or empty? At the moment the
> constructor will blindly accept a version for an unnamed module,
> and I am assuming this is wrong - am I right?

Good point.  The constructor should throw IAE if module name/version and class loader name is an empty string.

> toLoaderModuleClassName will call ClassLoader.getName().
> If a class loader overrides getName() then that might be a different
> name than what StackTraceElement.getClassLoaderName() returns.
> Is that an issue?

I added a test to verify StackTraceElement that uses the ClassLoader's name field.  I also added @apiNote in ClassLoader::getName.

> Also I wonder whether you should be considering including a fix
> for as part
> of this change (though arguably the bug has been there since
> new fields were added to StackTraceElement in 9).

It is related but it’s better to separate it from this issue.  Do you have cycle to help write a test case?  I can help fix JDK-8167099 later.


More information about the hotspot-runtime-dev mailing list