RFR 8145964: NoClassDefFound error in transforming lambdas

[David Holmes]

On 11/08/2016 10:09 PM, Coleen Phillimore wrote:
>
>
> On 8/10/16 8:37 PM, David Holmes wrote:
>> Hi Coleen,
>>
>> On 11/08/2016 12:45 AM, Coleen Phillimore wrote:
>>>
>>> New webrev:
>>>
>>>
>>> http://cr.openjdk.java.net/~coleenp/8145964.02/webrev/index.html
>>
>> First I'm very surprised that the existing logic in
>> JvmtiEnv::RetransformClasses doesn't utilize
>> VM_RedefineClasses::is_modifiable_class to determine when to return
>> JVMTI_ERROR_UNMODIFIABLE_CLASS. It would seem easy for the two bits of
>> code to get out of sync!
>>
>> Second, why do you silently ignore an attempt to redefine an anonymous
>> class instead of returning JVMTI_ERROR_UNMODIFIABLE_CLASS? The
>> restriction on transforming anonymous classes seems no different to me
>> to the restriction on transforming primitive or array classes.
>
> I believe that there are existing applications, as in the test, get all
> the loaded classes and try to transform them.  These will get the VM
> anonymous class, so we didn't want to give them an error. Or set CLFH
> and vm anonymous classes fall into the load hook. There's another bug
> that Rachel has that ignores them for CFLH (rather than crashing).
>
> From the Java standpoint, the existence of vm anonymous classes are a
> implementation detail and not real classes, and it's better to hide
> these as much as possible.

I think this is a mistake. While anonymous classes may have started as 
some obscure VM implementation detail related to JSR-292 and 
InvokeDynamic they are now entities that are prevalent in a running Java 
application due to the extensive use of Methodhandles and their use by 
lambda expressions. Unless we can hide such classes completely (we 
can't!) we need to define their semantics when treated like other 
"normal" Java classes. This should have been addressed for JVM TI when 
anonymous classes came into existence but it wasn't. But we have to deal 
with it, so simply define them as non-modifiable classes just like array 
classes and primitives. If code gets an error that is good it _needs_ to 
get an error because it needs to realize that it is dealing with 
anonymous classes not "real" ones!

Cheers,
David

> Coleen
>
>>
>> Thanks,
>> David
>>
>>> Reran jvmti tests.
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 8/10/16 9:32 AM, Coleen Phillimore wrote:
>>>>
>>>>
>>>> On 8/9/16 7:40 PM, David Holmes wrote:
>>>>> Hi Coleen,
>>>>>
>>>>> On 10/08/2016 4:52 AM, Coleen Phillimore wrote:
>>>>>> Summary: Skip VM anonymous classes in retransformation and give an
>>>>>> error
>>>>>> for redefinition.
>>>>>>
>>>>>> Contributed by Tom Rodriguez.
>>>>>>
>>>>>> Tested with redefinition colocated tests (tonga) and
>>>>>> java/lang/instrument tests, and added test case.
>>>>>>
>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8145964.01/webrev
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8145964
>>>>>
>>>>> Shouldn't anonymous classes be added to the set of non-modifiable
>>>>> classes - so IsModifiableClass returns false and we just add this
>>>>> case to VM_RedefineClasses::is_modifiable_class.
>>>>
>>>> Yes, that might be a better way to do it.
>>>> thanks,
>>>> Coleen
>>>>
>>>>>
>>>>> ??
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>
>>>
>