RFR(M): 8165550: Add class loader names to ClassCastException message

[David Holmes]

On 12/11/2016 9:05 AM, David Holmes wrote:
> On 12/11/2016 8:58 AM, Mandy Chung wrote:
>>
>>> On Nov 11, 2016, at 3:13 AM, Daniel Fuchs <daniel.fuchs at oracle.com>
>>> wrote:
>>>
>>> Hi,
>>>
>>> On 10/11/16 23:24, David Holmes wrote:
>>>> Hi Rachel,
>>>>
>>>> On 11/11/2016 8:31 AM, Rachel Protacio wrote:
>>>>> Ah yes, my apologies - I should have provided more context. This is
>>>>> the
>>>>> VM corollary of Mandy's change
>>>>> http://hg.openjdk.java.net/jdk9/dev/hotspot/rev/48fce55afe71 which
>>>>> implements the new classloader name scheme and uses it in Stack
>>>>> Traces.
>>>>> In the fullest instance, it could look like
>>>>>    MyClassLoader/module1 at 9.0/package1.class1
>>>>> The class loader is not listed if it is built-in, the module is not
>>>>> listed if it is unnamed, the version is not listed if it is a
>>>>> "non-jdk"
>>>>> module.
>>>>
>>>> Ah I see. Though not at all clear why we would care about showing the
>>>> JDK module version ??
>>>
>>> Mandy will correct me if I'm wrong, but AFAIU, for JDK modules,
>>> the version is displayed only if the module is an upgradeable
>>> module. The relevant algorithm [1] that does that can be seen in
>>> StackTraceElement::toLoaderModuleClassName(Class<?>):
>>>
>>
>> That is correct and that’s the format we implemented for the stack trace.
>>
>> The VM does not have an easy way to determine if a JDK module is
>> upgradeable or not.  There are a couple options that we considered for
>> example include such information when defining the module.  The reason
>> to include a version in an upgradeable module so that from the stack
>> trace it can tell if the class is not the version in the JDK.   So we
>> came up with this option such that the version is omitted if it’s a
>> JDK module coming from the image that includes upgradeable modules
>> that are not upgraded.  If these upgradeable modules are loaded from
>> image, they are the same version as the runtime.  If the module is
>> upgraded, the version will be included.
>>
>> We agree to not to change the JVM_DefineModule interface since jake
>> changes are in flight.  The exception message will have a small
>> difference but really minor and still provides the useful information
>> for troubleshooting.
>
> But what is being proposed for the ClassCastException message is almost
> the complete opposite of what was done for the stacktrace. This needs to
> be reconciled IMO, even if the VM can't deal with the upgradeable part.

Okay there is a difference between the code and how Rachel described it. 
The code:

1977       // Use version if exists and is not a jdk module
1978       if (module->is_non_jdk_module() && module->version() != NULL) {

What Rachel said: "the version is not listed if it is a "non-jdk" module."

So I'll take the "not" as a typo and all is good. :)

Cheers,
David

> Thanks,
> David
>
>> Mandy
>>
>>>
>>> [1]
>>> http://hg.openjdk.java.net/jdk9/dev/jdk/file/5b6b8e24a20b/src/java.base/share/classes/java/lang/StackTraceElement.java#l414
>>>
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>>
>>