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

[serguei.spitsyn at oracle.com]

On 4/20/19 11:29, Daniel D. Daugherty wrote:
> On 4/16/19 5:40 AM, 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/
>
> src/hotspot/share/prims/jvmtiRedefineClasses.cpp
>     No comments.
>
> src/hotspot/share/runtime/arguments.cpp
>     No comments.
>
> src/hotspot/share/runtime/globals.hpp
>     No comments.
>
> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineAddLambdaExpression.java
> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineDeleteJmethod.java
> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineSubtractLambdaExpression.java
>     No comments.
>
> test/jdk/com/sun/jdi/RedefineAddPrivateMethod.java
>     No comments.
>
> test/jdk/com/sun/jdi/lib/jdb/Debuggee.java
>     No comments.
>
> test/jdk/com/sun/jdi/lib/jdb/JdbTest.java
>     No comments.
>
> test/jdk/java/lang/instrument/RedefineAddDeleteMethod/DeleteMethodHandle/MethodHandleDeletedMethod.java
> test/jdk/java/lang/instrument/RedefineMethodAddInvoke.sh
> test/jdk/java/lang/instrument/RedefineMethodDelInvoke.sh
> test/jdk/java/lang/instrument/RedefineMethodInBacktrace.sh
>     No comments.
>
> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/TestAddDeleteMethods.java
>     I like the new test! It gives me hope about converting
>     the old shell tests into java tests.
>
> Thumbs up!

Thank you a lot for the review, Dan!

>
> Update: I see the fix was already pushed. Sorry I didn't get
> to this review in time.

No worry.
It is is nice to get your thumbs up anyway!

Also, I want to say that I extremely appreciate the help from number of 
HotSpot engineers:
Coleen, David H. (especially, Coleen and David!), Karen, Alan, Mandy, 
Dan and V. Ivanov!
We had several rounds of discussions on this issue.
It is on our plans to make this analysis publicly available.

Thanks!
Serguei

>
> Dan
>
>>
>> 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
>>
>