RFR 8145964: NoClassDefFound error in transforming lambdas
Coleen Phillimore
coleen.phillimore at oracle.com
Wed Aug 17 17:57:42 UTC 2016
On 8/17/16 12:17 PM, Tom Rodriguez wrote:
>>>
>>> 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…
>
> I would certainly expect that to be the normal way of using it. I
> looked through the netbeans profiler, which is used in VisualVM which
> started my down this path in the first place, for some equivalent
> logic but was unable to find it. It’s possible I simply missed it
> since the code is fairly complex. I guess I would expect some clever
> Java programmers to just use isArray directly instead of calling
> isModifiable. I could perform the experiment of adding Coleens fix to
> our graal-jvmci-8 and see how VisualVM behaves, if that’s interesting.
> Currently the graal JDK build simply ignores requests to rewrite
> anonymous classes to work around this issue.
I thought I saw that VisualVM worked around this problem by excluding a
class by it's name, ie. if it included a '/'. IsModifiableClass call
seems like it would be more correct for them to change it to once this
change gets in.
Coleen
>
> tom
>
>>
>> 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