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

Mandy Chung mandy.chung at oracle.com
Wed Nov 2 19:11:19 UTC 2016


> On Nov 2, 2016, at 4:29 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> I'd suggest passing 'name' to 'checkCreateClassLoader()' and do
> this check in checkCreateClassLoader instead - in order to do
> the checks before 'this' is created.
> 

Fixed.

> That's not exactly what I had in mind. I don't have
> any particular feeling for whether "" should be allowed
> or not. Maybe it would be simpler to just allow "" to mean the
> same thing as 'null'. What I meant was:
> Can moduleVersion be non null (and non empty) if moduleName is null
> (or empty)? In other words can an unnamed module have a version?
> 
> Also if you add some validation to the constructor then
> you will need to add a readObject to duplicate the validation
> checks. And if you disallow "" then checking for isEmpty() in
> toString() is not needed. But I have the feeling that simply
> allowing "" and null to mean the same thing would be more
> robust. It's just the question of having a moduleVersion
> for an unnamed module that bothers me.
> 

Having a second thought - it can’t do much validation with these strings e.g. whether the class loader name is the one loading the class or the module where this class is in.  I think it’s better to keep it simple and allow empty string and StackTraceElement::toString should not print module version for unnamed module (I fixed a bug there. It was intended to do that anyway).  Would that address your concern?

> 
> Right - no problem with that.
> I haven't given any thought to a test case for JDK-8167099.
> I believe it will prevent to connect a JDK 9 jvisualvm to a
> JDK 8 process if not fixed though. Maybe serializing a ThreadInfo
> composite data containing some MonitorInfo in JDK 8, and
> then deserializing that in JDK 9 and trying to convert it
> back to ThreadInfo in 9 would show the issue.


We should fix that.

Updated version:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.04/

Mandy


More information about the hotspot-runtime-dev mailing list