RFC (csr): 8221528: Introduce compatibility mode with VM option -XX:AllowRedefinitionToAddOrDeleteMethods

[coleen.phillimore at oracle.com]

On 3/28/19 7:22 AM, David Holmes wrote:
> On 28/03/2019 8:32 pm, Thomas Stüfe wrote:
>> On Thu, Mar 28, 2019 at 11:10 AM David Holmes 
>> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>>
>>     On 28/03/2019 4:55 pm, Thomas Stüfe wrote:
>>      > On Thu, Mar 28, 2019 at 8:40 AM David Holmes
>>     <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>      > <mailto:david.holmes at oracle.com
>>     <mailto:david.holmes at oracle.com>>> wrote:
>>      >
>>      >     Hi Thomas,
>>      >
>>      >     On 28/03/2019 5:02 pm, Thomas Stüfe wrote:
>>      >      > Hi Serguei,
>>      >      >
>>      >      > This is planned to be introduced in jdk 13?
>>      >      >
>>      >      > The only concern I have is that both paths (old and new
>>     behavior)
>>      >     should
>>      >      > be well tested, and hiding the old behavior behind an 
>> optional
>>      >     switch
>>      >      > may reduce its test coverage considerably.
>>      >
>>      >     The old behaviour is hardly ever used - I'm not even sure we
>>     currently
>>      >     have a test that tries to use it - so it's already not "well
>>     tested"
>>      >     and
>>      >     there's no intent here of increasing test coverage for the
>>     code we plan
>>      >     to remove. The new path will be tested as we do have a 
>> test that
>>      >     expects
>>      >     to get the error. And Serguei will of course have a simple
>>     test that
>>      >     checks the flag with both values.
>>      >
>>      >
>>      > I remember at least one case causing crashes in our code base
>>     where the
>>      > method ordering array was not reordered on RedefineClasses. 
>> May have
>>      > been this one: JDK-8149743.
>>      >
>>      > My point is, I have seen crashes in the field coming from 
>> bytecode
>>      > agents redefining classes with method added or substracted, so it
>>     is no
>>      > theoretical problem.
>>
>>     My point is that we don't really care if the code works well or not,
>>     we're intending to kill it off not improve it. It's a capability 
>> that
>>     really should never have snuck in to the code.
>>
>>
>> If you provide a backward compatibility switch which is not well 
>> tested, the coding behind it may bitrot; then, when you want to use 
>> it, you would run into errors. That is worse than not having that 
>> switch.
>
> AFAIK we don't actively test this code currently, so in that sense it 
> can bitrot even without the flag. Serguei can correct me if I am wrong 
> here.

This is not true.  We do actively test this code and I've personally 
fixed many bugs in this.  We will continue to have the tests we have 
now, with the option, to provide continued support until the option is 
removed.

Thanks,
Coleen

>
> David
> -----
>
>> Jdk head development goes at quite a pace, the chance that something 
>> which is not tested well bitrots is not that absurd.
>>
>> I am not idly bike shedding. I remember distinctly running into 
>> problems associated with RedefineClasses method adding/removal (I 
>> even remember us talking about it). So I know this is a thing which 
>> can happen. I do not want to improve it but I would like it to 
>> reliably work for as long as this switch exists.
>>
>> Cheers, Thomas
>>
>>     Cheers,
>>     David
>>