Re: RFR: 8141341: CDS should be disabled if JvmtiExport::should_post_class_file_load_hook() is true

[Ioi Lam]

(Adding serviceability-dev@openjdk.java.net)

Hi Jiangli,
I think the two blocks of

    if (RequireSharedSpaces) {
      tty->print_cr("An error has occurred while loading shared class.");
      tty->print_cr("Tool agent requires sharing to be disabled.");
      vm_exit(1);
    }

should be removed.

If the agent is specified in the command line, the JVM start would be 
aborted. This is already handled by your changes to 
src/share/vm/memory/metaspace.cpp

If the agent is attached later, after the VM has started, I think we 
should not quit the VM. Consider this scenario -- a production server 
could be running for a long time without issues using -Xshare:on, then 
when the user attaches a diagnostic agent the JVM suddenly quits.  I 
think it's better to cause the agent attachment to fail with something like

jvmtiError
JvmtiEnv::SetEventNotificationMode(jvmtiEventMode mode, jvmtiEvent 
event_type, jthread event_thread,   ...) {

...
  if (event_type == JVMTI_EVENT_CLASS_FILE_LOAD_HOOK && enabled) {
+   if (RequireSharedSpaces && !universe::is_bootstraping()) {
+     tty->print_cr("Tool agent that requires 
JVMTI_EVENT_CLASS_FILE_LOAD_HOOK cannot be dynamically loaded after JVM 
has started");
+     return JVMTI_ERROR_ILLEGAL_ARGUMENT;
+   }
    record_class_file_load_hook_enabled();
  }
...
}


Thanks
- Ioi



On 7/7/16 6:08 PM, Jiangli Zhou wrote:
Please review the following webrev that disables CDS when 
JvmtiExport::should_post_class_file_load_hook() is enabled at runtime.

   webrev: http://cr.openjdk.java.net/~jiangli/8141341/webrev.00/
   bug: JDK-8141341 <https://bugs.openjdk.java.net/browse/JDK-8141341>

With -Xshare:on
If -Xshare:on is used and JvmtiExport::should_post_class_file_load_hook() is 
required, the VM now reports "Tool agent requires sharing to be disabled” and 
terminates. This is the same behavior as jdk8.

With -Xshare:auto
The change in meatspace.cpp is to detect the case where 
JvmtiExport::should_post_class_file_load_hook() is enabled very early during 
JVM initialization. In that case, we disable CDS entirely, including all shared 
classes, symbols, and Strings objects.

The change in systemDictionary.cpp is to detect the cases where 
JvmtiExport::should_post_class_file_load_hook() is enabled late, which include 
agent dynamically attached at runtime. In those cases, JVM does not allow 
loading classes from the shared archive. The shared symbols and Strings can 
still be used.

Thanks,
Jiangli