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

[David Holmes]

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

Cheers,
David

> Cheers Thomas
> 
> 
>     Cheers,
>     David
> 
> 
>      > Cheers, Thomas
>      >
>      > (p.s. html format does not work well with the OpenJDK mailing list
>      > archive, see
>      >
>     https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-March/027612.html)
>      >
>      > On Thu, Mar 28, 2019 at 1:57 AM serguei.spitsyn at oracle.com
>     <mailto:serguei.spitsyn at oracle.com>
>      > <mailto:serguei.spitsyn at oracle.com
>     <mailto:serguei.spitsyn at oracle.com>> <serguei.spitsyn at oracle.com
>     <mailto:serguei.spitsyn at oracle.com>
>      > <mailto:serguei.spitsyn at oracle.com
>     <mailto:serguei.spitsyn at oracle.com>>> wrote:
>      >
>      >     Hi All,
>      >
>      >     This is request for comments and potentially reviews for the
>      >     following CSR:
>      > https://bugs.openjdk.java.net/browse/JDK-8221528
>      >
>      >
>      >     *Problem*
>      >
>      >     Now, the implementation of JVMTI RedefineClasses and
>      >     RetransformClasses allows to
>      >     add/delete private static and private final instance methods
>     in the
>      >     new class versions.
>      >
>      >     It means that current implementation does not follow the JVM
>     TI spec
>      >     which explicitly states:
>      >
>      >     "The redefinition must not add, remove or rename fields or
>     methods,
>      >     change the signatures
>      >       of methods, change modifiers, or change inheritance."
>      >
>      >     For details, please, see the spec:
>      >
>      >
>     https://docs.oracle.com/en/java/javase/12/docs/specs/jvmti.html#RedefineClasses
>      >
>     https://docs.oracle.com/en/java/javase/12/docs/specs/jvmti.html#RetransformClasses
>      >
>      >     The decision was made to align the implementation with the spec.
>      >     For reference, please, see the
>      > https://bugs.openjdk.java.net/browse/JDK-8192936.
>      >
>      >     This decision is going to cause some inconveniences to the
>     customers.
>      >
>      >
>      >     *Solution*
>      >
>      >     The solution is to introduce a compatibility mode with new
>      >     command-line VM option:
>      >         -XX:{+|-}AllowRedefinitionToAddOrDeleteMethods
>      >
>      >     New option will enable old behavior and allow 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 will be disabled.
>      >
>      >     New option is deprecated right away.
>      >     The plan is to keep it for a couple of releases to allow
>     customers
>      >     (tool vendors)
>      >     to remove dependency on old behavior from their tools.
>      >
>      >     Thanks,
>      >     Serguei
>      >
>