RFR: 8149743: JVM crash after debugger hotswap with lambdas

[Daniel D. Daugherty]

I was originally not going to continue on this part of the review
thread. However, after I reviewed the webrev, I feel that some of
this reply must be addressed.


On 2/22/16 4:53 PM, Coleen Phillimore wrote:
>
>
> On 2/22/16 6:46 PM, Daniel D. Daugherty wrote:
>> Just noticed that this thread did not include the Serviceability
>> aliases... JVM/TI belongs to the Serviceability team so...
>>
>> Dan
>>
>>
>> On 2/22/16 4:44 PM, Daniel D. Daugherty wrote:
>>> On 2/22/16 1:28 PM, Coleen Phillimore wrote:
>>>>
>>>> This fix looks good.   The test looks like it uses some framework I 
>>>> don't know, in a directory that I don't usually run tests in.
>>>>
>>>> We have some redefinition tests in 
>>>> hotspot/test/runtime/RedefineTests - can you write one with source 
>>>> code there instead?
>>>
>>> The redefine tests in the runtime directory are in the wrong place.
>
> The tests in the runtime directory test runtime changes to support 
> redefinition primarily, so are in the right place.

This fix is 3 code lines in VM_RedefineClasses::redefine_single_class()
in hotspot/src/share/vm/prims/jvmtiRedefineClasses.cpp which is the core
function for JVM/TI RedefineClasses(). jvmtiRedefineClasses.cpp is not
Runtime code nor are the 3 code lines a runtime change.

Please explain how this fix fits your criteria above for putting a
RedefineClasses() test in the runtime directory?


> They are run with JPRT -testset hotspot.

The tests included in '-testset hotspot' are defined by the teams
working on hotspot. The Serviceability team is able to add or
exclude tests from their part of '-testset hotspot'.


> They don't use these other frameworks intentionally.

Why? What is so special about these tests that they cannot or should not
use existing testing frameworks? Also, why would a Serviceability team
person even look for RedefineClasses() tests in hotspot/test/runtime?


>
>>>
>>> JVM/TI RedefineClasses() is a debugging API and jdk/test/com/sun/jdi
>>> is the usual place for JDI tests that exercise RedefineClasses(). If
>>> the test is using the Java entry points (JLI/JPLIS), then those tests
>>> belong in jdk/test/java/lang/instrument. Of course, if the test needs
>>> to use to JDWP entry point for JVM/TI RedefineClasses(), then those
>>> tests live in yet another location.
>>>
>
> Can you or someone from serviceability team review this test then?

I just reviewed it, but Serguei already reviewed on 2016.02.19.


> I have no idea what it does

Since you work on RedefineClasses, it would be a good idea to
familiarize yourself with all of the different ways that API
gets called.


> and would prefer a test more tailored to testing the JVM functionality 
> than jdb commands.

That's not typically how Serviceability regression tests are created.
JPDA is a stack of protocols and there are tests targeted at each
layer of the protocol stack:

   hprof and jdb        Tonga and JTREG tests
   JDI and JLI/JPLIS    Tonga and JTREG tests
   JDWP                 Tonga
   JVM/TI               Tonga and a few JTREG tests

Each layer has a way to make a RedefineClasses() call and a
regression test is typically written at the layer where the
bug can be easily reproduced. In this bug's case, the customer
provided a jdb session scenario which was easy to turn into
a JTREG test in jdk/test/com/sun/jdi. There are plenty of
examples to copy and modify.

Dan



>
> Thanks,
> Coleen
>>> Dan
>>>
>>>
>>>>
>>>> I linked an old bug to this, can you see if they are the same?
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 2/18/16 11:11 AM, Andreas Eriksson wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this fix for JDK-8149743: JVM crash after debugger 
>>>>> hotswap with lambdas
>>>>> https://bugs.openjdk.java.net/browse/JDK-8149743
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~aeriksso/8149743/webrev.00/
>>>>>
>>>>> When redefining a class to add or delete methods an array that's 
>>>>> tracking method ordering is not updated correctly.
>>>>> This change swaps the method ordering array between the old class 
>>>>> being redefined and the scratch class it is being redefined into 
>>>>> at the same point where we swap the methods and constant pool 
>>>>> between them.
>>>>>
>>>>> Regards,
>>>>> Andreas
>>>>
>>>
>>>
>>
>
>