8358890: VM option -XX:AllowRedefinitionToAddDeleteMethods should be obsoleted then expired #26232
Conversation
…obsoleted then expired
|
👋 Welcome back sspitsyn! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@sspitsyn The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
There was a problem hiding this comment.
This involved a lot more code than I had envisaged.
The test situation seems problematic as it suggests to me that we have required functionality (redefine a method containing a lambda expression) that is not possible without the flag. And it seems this flag is also being used in the wild e.g.
https://blog.picnic.nl/embracing-java-17-heres-what-we-learned-69779d95fdf2
As of Java 16, JDK internals are strongly encapsulated by default (JEP 396). These and other changes mean that some dependencies now require additional JVM flags such as --add-opens and -XX:+AllowRedefinitionToAddDeleteMethods to function properly. We updated our shared build system such that teams can configure these flags in a single place, ensuring that test and production runtimes remain in sync.
With more info in reactor/BlockHound#33
So I am quite concerned that this "workaround" has become entrenched.
| @@ -4114,18 +4050,14 @@ void VM_RedefineClasses::flush_dependent_code() { | |||
| JvmtiExport::set_all_dependencies_are_recorded(true); | |||
| } | |||
|
|
|||
| void VM_RedefineClasses::compute_added_deleted_matching_methods() { | |||
| void VM_RedefineClasses::compute_matching_methods() { | |||
There was a problem hiding this comment.
I can't see that this method actually still does anything useful. ??
| @@ -522,7 +522,7 @@ void Arguments::init_version_specific_system_properties() { | |||
| static SpecialFlag const special_jvm_flags[] = { | |||
| // -------------- Deprecated Flags -------------- | |||
| // --- Non-alias flags - sorted by obsolete_in then expired_in: | |||
| { "AllowRedefinitionToAddDeleteMethods", JDK_Version::jdk(13), JDK_Version::undefined(), JDK_Version::undefined() }, | |||
| { "AllowRedefinitionToAddDeleteMethods", JDK_Version::jdk(13), JDK_Version::jdk(26), JDK_Version::jdk(27) }, | |||
There was a problem hiding this comment.
Please move this line to before line 535 (ParallelRefProcEnabled) so that we are sorted on obsolete_in as required.
|
/label remove core-libs |
|
@AlanBateman |
The VM option -XX:AllowRedefinitionToAddDeleteMethods was added in JDK 13 as a temporary backward compatibility flag under JDK-8192936 and was immediately marked as Deprecate. The fix is to obsolete this option in JDK 26 and expire in JDK 27.
TBD: Need to submit a related CSR.
There are two concerns which may require some negotiation with the Runtime (@coleenp @dcubed-ojdk @dholmes-ora) and SQE (@lmesnik) teams:
Testing:
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26232/head:pull/26232$ git checkout pull/26232Update a local copy of the PR:
$ git checkout pull/26232$ git pull https://git.openjdk.org/jdk.git pull/26232/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 26232View PR using the GUI difftool:
$ git pr show -t 26232Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26232.diff
Using Webrev
Link to Webrev Comment