RFR(S): 8192936: RI does not follow the JVMTI RedefineClasses spec that is too strict in the definition

[serguei.spitsyn at oracle.com]

Hi David,

Thank you a lot for review!
All suggestions are good. I'll file new bug to cover them.

Thanks,
Serguei


On 4/22/19 17:44, David Holmes wrote:
> Hi Serguei,
>
> A belated LGTM - sorry I didn't get to this sooner.
>
> Three minor comments:
>
> 1. I would have suggested to add "(Deprecated)" to the description of 
> the new flag in globals.hpp
>
> 2. The new flag should have been added to the deprecated VM options 
> tests.
>
> 3. The new test should run in both a positive and negative mode so 
> that it also checks that the new flag works.
>
> Thanks for taking care of this - it has been a hard slog. :)
>
> David
> -----
>
> On 16/04/2019 7:40 pm, serguei.spitsyn at oracle.com wrote:
>> Please, review the fix of:
>> https://bugs.openjdk.java.net/browse/JDK-8192936
>>
>>
>> Webrev (fix from Coleen):
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8192936-redef-add-delete.1/ 
>>
>>
>> I've already reviewed and updated the webrev with my suggestions.
>>
>>
>> Reviewed and approved CSR:
>> https://bugs.openjdk.java.net/browse/JDK-8221528
>>
>>
>> Summary:
>>    The fix introduces new VM option 
>> -XX:AllowRedefinitionToAddOrDeleteMethods
>>    for compatibility with previous releases.New option enables old 
>> behavior
>>    and allows the JVM TI RedefineClasses and RetransformClasses to 
>> add/delete
>>    private static and private final instance methods in the new class 
>> versions.
>>    Without this option the old behavior is disabled.
>>
>>    New option is deprecated right away.
>>    The plan is to keep this option for several releases to allow 
>> customers
>>    (tool vendors) to remove dependency on old behavior from their tools.
>>
>>
>> Testing:
>>    Added new test to verify that class redefinitions which add or 
>> delete methods
>>    return expected JVMTI error codes:
>> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/TestAddDeleteMethods.java 
>>
>>
>>    Several jvmti, com/sun/jdi and java/lang/instrument tests which 
>> need old behavior are updated to use new flag.
>>
>>    Run locally on Linux-x64 the following test suites in release and 
>> fastdebug mode:
>> - open/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/
>>      - vmTestbase_nsk_jvmti
>>      - vmTestbase_nsk_jdi
>>      - vmTestbase_nsk_jdb
>>      - vmTestbase_nsk_jdwp
>>      - jdk_jdi
>>      - jdk_instrument
>>
>>    Submission of corresponding mach5 jobs is in progress.
>>
>> Thanks,
>> Serguei
>>