RFR(S): 8203329: JDWP/JDI VM information string is incorrect

[David Holmes]

Hi Chris,

Great sleuthing on this one and thanks for explaining things for me in 
the bug report!

The fix you have addresses the problem observed with the "vm info" 
string. One small request, can you update this comment in thread.cpp please:

  // We need this for ClassDataSharing - the initial vm.info property is set
   // with the default value of CDS "sharing" which may be reset through
   // command line options.
   reset_vm_info_property(CHECK_JNI_ERR);

As we actually need that for any piece of information that might be used 
in the vm info string initially, that can subsequently change due to 
option processing e.g. UseSharedSpaces and UseAOT. Suggestion:

// We need this to update the vm.info property in case any flags used
// to initially define it have been changed with command-line options.
reset_vm_info_property(CHECK_JNI_ERR);

Thanks!

---

That said I think we can simplify things somewhat now you discovered the 
bug in reset_vm_info_property. If we simply update the native vm_info 
property as soon as we've completed argument parsing, then we will have 
the right value in place for copying to the Java level during 
java.lang.System initialization. We don't need any of the existing code 
in reset_vm_info because we copy the correct vm_info in the first place. 
In short you should be able to fix this with simply:

   jint parse_result = Arguments::parse(args);
   if (parse_result != JNI_OK) return parse_result;

   os::init_before_ergo();

   jint ergo_result = Arguments::apply_ergo();
   if (ergo_result != JNI_OK) return ergo_result;

+ const char *vm_info = VM_Version::vm_info_string();
+ Arguments::PropertyList_update_value(Arguments::system_properties(), 
"java.vm.info", vm_info);
...
-  // We need this for ClassDataSharing - the initial vm.info property 
is set
-  // with the default value of CDS "sharing" which may be reset through
-  // command line options.
-  reset_vm_info_property(CHECK_JNI_ERR);

and delete reset_vm_info_property.

Just a thought - feel free to defer - it's really a runtime issue anyway. :)

Thanks,
David

On 7/06/2018 8:24 AM, Chris Plummer wrote:
> Hello,
> 
> Please review the following:
> 
> https://bugs.openjdk.java.net/browse/JDK-8203329
> http://cr.openjdk.java.net/~cjplummer/8203329/webrev.00/
> 
> The native version of the java.vm.info property was getting out of sync 
> with the java version. Details can be found here:
> 
> https://bugs.openjdk.java.net/browse/JDK-8203329?focusedCommentId=14185679&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14185679 
> 
> 
> thanks,
> 
> Chris