It was my assumption that the reason adding private methods were allowed with redefineClass was to support adding/removing lambdas from method bodies, since javac converts those into private methods.
Removing this capability would remove this support. I would assume that would cause users a lot of headache, since from a user's point-of-view, they are just changing the method body, which should be supported! /Michael From: serviceability-dev <[email protected]> on behalf of David Holmes <[email protected]> Sent: 28 March 2019 12:10 To: Thomas Stüfe Cc: [email protected]; Coleen Phillimore Subject: Re: RFC (csr): 8221528: Introduce compatibility mode with VM option -XX:AllowRedefinitionToAddOrDeleteMethods On 28/03/2019 4:55 pm, Thomas Stüfe wrote: > On Thu, Mar 28, 2019 at 8:40 AM David Holmes <[email protected] > <mailto:[email protected]>> 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 [email protected] > <mailto:[email protected]> > > <mailto:[email protected] > <mailto:[email protected]>> <[email protected] > <mailto:[email protected]> > > <mailto:[email protected] > <mailto:[email protected]>>> 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 > > >
