[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(S): 8192936: RI does not follow the JVMTI RedefineClasses spec that is too strict in the def
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2019-04-23 9:28:27
Message-ID: 97fcebc4-01fb-72c0-3334-5a4e73d91018 () oracle ! com
[Download RAW message or body]

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

[prev in list] [next in list] [prev in thread] [next in thread]