RFR 8145964: NoClassDefFound error in transforming lambdas

Tom Rodriguez tom.rodriguez at oracle.com
Tue Aug 16 19:57:40 UTC 2016 

>> 
>> Sorry, I was away on vacation so I couldn’t respond to this earlier.  In principle I agree with David that from an API perspective anonymous classes should just be treated as unmodifiable.  I don’t completely understand the compatibility issue this presents though.  The only way an agent should be encountering them is if it gets all loaded classes and then passes that result back through redefine classes.  Presumably they must be filtering out the array classes from that list already.  If they are using IsModifiable for this then it would all work out great but they could just be filtering on isArray, which seems most likely to me.
> 
> That's good.  I'm glad that there isn't a compatibility problem.  I guess there wouldn't be since the reported applications got NoClassDefFoundError, now they'll get an UnmodifiableClassException but can call isModifiableClass first anyway.

I wouldn’t say there isn’t a compatibility problem, just that if they encounter it they already had a problem, so it’s trading one problem for another.  It’s unfortunate that the JVMCI define classes path doesn’t have clearer error reporting.  If you pass in a bunch of classes and one gets the unmodifiable class error, there’s no indication of which class actually had the error.  Not much we can do about that though.

tom

> 
>> 
>> Given that the anonymous classes become unusable after redefinition I’d have to assume that agents which encountered this problem have worked around it themselves.  The netbeans profiler worked about this by filtering out class names with a “/“ in them.  http://hg.netbeans.org/main-silver/rev/ed472a217546 <http://hg.netbeans.org/main-silver/rev/ed472a217546>  https://netbeans.org/bugzilla/show_bug.cgi?id=245840 <https://netbeans.org/bugzilla/show_bug.cgi?id=245840> This is fairly similar to silently ignoring them which is part of why I suggested that fix in my report.
>> 
>> So I think the right fix for 9 is to treat this an an API clarification and make them unmodifiable.  Any agent which isn’t already handling this themselves is broken but doesn’t know it yet.  I assume there’s no intent to fix this in 8?
>> 
> 
> Great.  I can probably backport it to 8, when we get the details sorted out wrt the spec.
> 
> thanks,
> Coleen
> 
>> tom
>> 
>>> 
>>>> Similar wording change in the jvmtiCapabilities table that lists
>>>> can_redefine_any_class. Of course, IsModifiableClass itself will
>>>> have to be updated to spell out the classes that can't be modified.
>>>> 
>>>> Looks like the spec wording for can_retransform_any_class is pretty
>>>> much wrong w.r.t. IsModifiableClass() stuff and will all need work.
>>>> Also looks like can_retransform_any_class wording is completely
>>>> missing from IsModifiableClass().
>>>> 
>>>> Sorry this is turning into a can of worms...
>>> 
>>> We may have to switch to option 2 and just silently ignore everything related to VMACs.
>>> 
>>> That said I don't think that we have to spell out every potential case where a class may be unmodifiable. It should be sufficient (as currently stated) that you can't redefine or retransform an unmodifiable class. The only thing that needs fixing in my opinion is the definition of can_redefine_any_class to not refer to primitives or arrays but simply to any class for which IsModifiableClass returns true!
>>> 
>>> can_redefine_any_class is really puzzling if you consider can_redefine_classes. It is far from clear to me what set of classes can_redefine_any_class gives access to that can_redefine_classes does not ??? What purpose do two capabilities serve here?
>>> 
>>> David
>>> -----
>>> 
>>>> Dan
>>>> 
>>>> 
>>>>> 
>>>>> 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 <http://cr.openjdk.java.net/%7Ecoleenp/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 <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 <http://cr.openjdk.java.net/%7Ecoleenp/8145964.01/webrev>
>>>>>>>>>>>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8145964 <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
>> 
>

More information about the hotspot-dev mailing list