RFR 8145964: NoClassDefFound error in transforming lambdas

Sat Aug 13 00:33:08 UTC 2016

On 8/12/16 7:51 PM, David Holmes wrote:
> Hi Coleen,
>
> On 13/08/2016 4:19 AM, Coleen Phillimore wrote:
>>
>> Okay, I've made retransforming a class an unmodifiable class error and
>> will give whoever complains your phone number.   I've run the change
>> through the redefinition tests in our colocated (our closed),
>> noncolocated (also oracle-only), jdk/test/java/lang/instrument and
>> jdk/test/com/sun/jdi testsets.
>>
>> Please see new webrev.
>>
>> http://cr.openjdk.java.net/~coleenp/8145964.03/webrev/index.html
>
> Thanks, this is my preferred solution.
>
> Nit:
>
> !   // Cannot redefine an anonymous class.  Retransform filters them out.
>
> The second sentence  is not needed now.

Thanks, David.   HIt reload, I had already changed that sentence but 
neglected to create a new webrev.
>
> I didn't fully grok the test as I'm not familiar the various pieces.
>
>
> You need a second reviewer anyway so hopefully John and Tom will still 
> chime in.

Okay, I'll wait but it would be nice for someone else on the team to review.

thanks,
Coleen
>
> Thanks,
> David
>
>> Thanks,
>> Coleen
>>
>> On 8/11/16 11:44 PM, David Holmes wrote:
>>> On 12/08/2016 12:23 AM, Coleen Phillimore wrote:
>>>> On 8/11/16 8:20 AM, David Holmes wrote:
>>>>> 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!
>>>>>
>>>>
>>>> I disagree, and please read the bug where the consensus is that we
>>>> should ignore these for retransformation.  Redefinition is a different
>>>> matter because you have to give the class with new bytecodes, which
>>>> would be very bad, and that's why I chose to give an error message.
>>>
>>> I don't read such a consensus in the bug report.
>>>
>>>> This is John Rose's answer to a similar question:
>>>>
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-January/038353.html 
>>>>
>>>>
>>>
>>> Yes I added that link to the bug report initially. It reinforces that
>>> retransform/redefinition of VMAC is not intended to work  so don't do
>>> that: "Please don't lead your users to rely on them."
>>>
>>> I read more in John's response that the code doing the transformations
>>> should be skipping the VMAC's, than that the VM code should ignore the
>>> VMAC's if passed in.
>>>
>>> The current situation is inconsistent and that is what I object to,
>>> most strenuously. When I suggested this be handled through
>>> VM_RedefineClasses::is_modifiable_class I was expecting the VMAC to be
>>> treated as an unmodifiable class in all cases/sense. It is not
>>> acceptable to me to have IsModifiableClass return false yet
>>> retransform does not report JVMTI_ERROR_UNMODIFIABLE_CLASS. There
>>> should be tests that check that the two are consistent!
>>>
>>> Please, either silently ignore it everywhere, or else deal with it
>>> consistently as unmodifiable.
>>>
>>>> I am not willing to break existing applications.
>>>
>>> Any existing application that retransforms an anonymous class is
>>> already in unchartered waters. To quote John "Even if you were able to
>>> 'transform' one of these classfiles, it wouldn't necessary do what you
>>> think it should do". So if it is lucky any existing code that attempts
>>> to transform an anonymous class simply does no harm - but more likely
>>> it gets an error (as per this bug report), or worse it "does something
>>> you didn't think it would do"! If such code is now run against JDK 9
>>> then I think they would be grateful to be told that they are running
>>> broken code, and exactly where it is broken.
>>>
>>> Cheers,
>>> David
>>>
>>>> Coleen
>>>>
>>>>> 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
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>