RFR 8145964: NoClassDefFound error in transforming lambdas

Daniel D. Daugherty daniel.daugherty at oracle.com
Tue Aug 16 22:00:00 UTC 2016 

On 8/16/16 1:57 PM, Tom Rodriguez wrote:
>>>
>>> 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.

When the agent assembles the array of classes, it is supposed to
call IsModifiableClass() on each class before it adds the class
to the array...

Dan


>
> 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 
>>> 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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 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