RFR 8145964: NoClassDefFound error in transforming lambdas
Daniel D. Daugherty
daniel.daugherty at oracle.com
Thu Aug 18 02:18:15 UTC 2016
On 8/17/16 6:41 PM, David Holmes wrote:
> Just following up on discussion ...
>
> On 17/08/2016 11:55 AM, Coleen Phillimore wrote:
>>
>> Thanks, David.
>>
>> On 8/16/16 9:30 PM, David Holmes wrote:
>>> Hi Coleen,
>>>
>>> On 17/08/2016 9:16 AM, Coleen Phillimore wrote:
>>>> On 8/15/16 10:01 PM, David Holmes wrote:
>>>>> On 16/08/2016 9:34 AM, Coleen Phillimore wrote:
>>>>>> Rereading this and looking at the JVMTI spec, I think Dan's wording
>>>>>> and
>>>>>> changes make the most sense, if we are to keep IsModifiableClass to
>>>>>> exclude retransforming classes, with the exception of specifying
>>>>>> anonymous classes. I think the wording should just say "some
>>>>>> implementation defined classes" as below.
>>>>>
>>>>> I'm not sure what you mean exactly. Dan's changes seemed excessive to
>>>>> me. The only changes I see as essential are expressing the
>>>>> can_redefine/restransform_* in terms of IsModifiableClass.
>>>>
>>>> David, I did three things. I changed the description for the
>>>> capabilities can_redefine_any_class and can_retransform_any_class to:
>>>>
>>>> http://cr.openjdk.java.net/~coleenp/jvmti.html#jvmtiCapabilities.can_redefine_any_class
>>>>
>>>>
>>>
>>> Ok.
>>>
>>>> http://cr.openjdk.java.net/~coleenp/jvmti.html#jvmtiCapabilities.can_retransform_any_class
>>>>
>>>>
>>>
>>> Ok.
>>>
>>>> And added "some implementation defined classes" to the description of
>>>> IsModifiableClass
>>>>
>>>> http://cr.openjdk.java.net/~coleenp/jvmti.html#IsModifiableClass
>>>
>>> Typo: and some some implementation
>>
>> Good eye. I generated this file at least 10 times and didn't see this.
>>>
>>> What I dislike about this change is that it immediately raises the
>>> unanswered question: 'what implementation defined classes?'
>>
>> Yeah, I was going to say "internal vm implementation classes" but
>> anonymous classes aren't technically internal to the vm since you can
>> call the unsafe api from java. So I was sort of at a loss how to
>> describe this better.
>
> And that is really the problem - once you say something you have to
> say more to explain that something and it just gets out of hand.
>
>>>
>>> In re-reading the spec there is something else that now bothers me
>>> about this. The "any" capability is listed as an optional capability,
>>> which suggests to me that there are supposed to be circumstances where
>>> IsModifiableClass(x) may return false if you don't have the "any"
>>> capability but true if you do. To use Dan's example a class in the
>>> shared archive may not be modifiable, unless you have the "any'
>>> capability - but of course there is nothing that documents any such
>>> behaviour (and I'm not saying that is how hotspot works, or should
>>> work, only that the spec suggests something like this may be possible).
>>
>> Honestly, I'm pretty stumped by the concept of the _any_ class
>> capability or what it means. I don't see the jvm doing anything
>> different for _any_.
>
> Right - there seems to be a mismatch between what the spec allowed for
> and what hotspot provides. Arguably the spec was to allow for
> implementations less capable than hotspot that required explicit use
> of capabilities for some use cases. But the role of these capabilities
> is far from clear, and certainly there is no documentation as to how
> hotspot deals with the different capabilities.
Not necessarily less capable.
Imagine a JVM that has a mix of ROMized classes and RAM based classes.
The ROMized classes won't be redefineable or retransformable by default
in that development environment because you want to run the way the
system is deployed. However, you might want to debug those ROMized
classes without having shutdown the system, reburn the ROM and
restart the system... so you add a flag to your debugger invocation
that acquires the can_redefine_any_class capability on start-up.
When that capability is enabled, your ROMized classes are overlayed
with a COW mapping to anonymous memory. You can now debug the ROMized
code in a fix-and-continue debug session... of course, your debugger
is going to have to be smart enough to save off the modified pages
someplace so that you can know what you changed...
Dan
>
>>>
>>> On a related note, but should be covered by 8163973, the "Class File
>>> Load Hook" section also needs to clarify that CFLH events may not be
>>> generated for some implementation defined classes - though arguably
>>> the existing text:
>>>
>>> "Some classes might not be compatible with the function (eg. ROMized
>>> classes) and this event will not be generated for these classes. "
>>>
>>> might cover that sufficiently.
>>
>> Maybe we could add "Some classes might not be compatible with the
>> function (eg. ROMized classes or implementation defined classes) and
>> this event will not be generated for these classes. " as part of
>> Rachels' bug?
>
> Yes that is probably the right way to go.
>
> Thanks,
> David
>
>>>
>>>> I also rewrote the test. See webrev:
>>>>
>>>> http://cr.openjdk.java.net/~coleenp/8145964.04/webrev/index.html
>>>>
>>>> What do you think?
>>>
>>> Ship it!
>>
>> Thanks!
>> Coleen
>>>
>>> Thanks,
>>> David
>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>>
>>>>> David
>>>>>
>>>>>> thanks,
>>>>>> Coleen
>>>>>>
>>>>>> On 8/14/16 10:20 PM, Daniel D. Daugherty wrote:
>>>>>>> On 8/12/16 6:33 PM, Coleen Phillimore wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> 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.
>>>>>>>
>>>>>>> > http://cr.openjdk.java.net/~coleenp/8145964.03/webrev/index.html
>>>>>>>
>>>>>>> src/share/vm/prims/jvmtiEnv.cpp
>>>>>>> No comments.
>>>>>>>
>>>>>>> src/share/vm/prims/jvmtiRedefineClasses.cpp
>>>>>>> L133: // classes for primitives and arrays cannot be
>>>>>>> redefined
>>>>>>> That comment needs updating.
>>>>>>>
>>>>>>> test/runtime/RedefineTests/RetransformAnonymous.java
>>>>>>> L39: why is there a blank line here (in the middle of the
>>>>>>> imports)?
>>>>>>>
>>>>>>> L79: System.out.println("retransform called for " + name);
>>>>>>> Perhaps add "unexpected" to that message?
>>>>>>>
>>>>>>> Shouldn't there also be a test for RedefineClasses()?
>>>>>>>
>>>>>>>
>>>>>>> The JVM/TI spec will need updates also:
>>>>>>>
>>>>>>> - RedefineClasses() API
>>>>>>> - RetransformClasses() API
>>>>>>> - IsModifiableClass() API
>>>>>>>
>>>>>>> Here's the line from IsModifiableClass():
>>>>>>>
>>>>>>> > Primitive classes (for example, java.lang.Integer.TYPE) and array
>>>>>>> classes are never modifiable.
>>>>>>>
>>>>>>> Perhaps change it like this:
>>>>>>>
>>>>>>> Primitive classes (for example, java.lang.Integer.TYPE), array
>>>>>>> classes, and
>>>>>>> some implementation defined classes (for example, anonymous
>>>>>>> classes)
>>>>>>> are
>>>>>>> never modifiable.
>>>>>>>
>>>>>>> Similar wording changes should be done for RedefineClasses() and
>>>>>>> RetransformClasses() APIs.
>>>>>>>
>>>>>>> There are various places that refer to can_redefine_any_class.
>>>>>>> Those
>>>>>>> places
>>>>>>> also need updating. Here's the one from RedefineClasses:
>>>>>>>
>>>>>>> > can_redefine_any_class Can modify (retransform or redefine)
>>>>>>> any
>>>>>>> non-primitive
>>>>>>> > non-array class. See IsModifiableClass.
>>>>>>>
>>>>>>> Should probably be changed to something like:
>>>>>>>
>>>>>>> can_redefine_any_class Can modify (retransform or redefine) any
>>>>>>> class except for
>>>>>>> a few specific classes. See
>>>>>>> IsModifiableClass for the classes
>>>>>>> that cannot be modified.
>>>>>>>
>>>>>>> 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...
>>>>>>>
>>>>>>> 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
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>
More information about the hotspot-dev
mailing list