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

[Ioi Lam]

(Adding serviceability-dev at 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
>
>