> On Nov 2, 2016, at 4:29 AM, Daniel Fuchs <daniel.fu...@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
