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

[serguei.spitsyn at oracle.com]

On 3/28/19 10:14, coleen.phillimore at oracle.com wrote:
>
>
> 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.

Yes, I'd like to confirm that we are going to test the switch with the 
tests we have now.
But we already discovered a couple of holes that we are not going to fix.

Thanks,
Serguei


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