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
