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

[David Holmes]

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
>