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

[David Holmes]

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.

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
>