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

[Jiangli Zhou]

Hi Ioi,

Thanks for the comments.

> On Jul 7, 2016, at 6:56 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> (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

I noticed in some cases the JvmtiExport::should_post_class_file_load_hook is not enabled very early during VM initialization, but rather at a later time for agent specified in the command line. When agent has SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_CLASS_FILE_LOAD_HOOK…), the enabling of the JvmtiExport::should_post_class_file_load_hook is done early and can be handled by the check in meatspace.cpp. In other case, the capability is enabled later, which is after the mapping of the shared spaces and initializing of the preloaded classes. Here is the stack trace of the later case:

#0  JvmtiExport::set_should_post_class_file_load_hook(bool) ()
#1  0x00007ffff617fe3e in JvmtiEventControllerPrivate::recompute_enabled() ()
#2  0x00007ffff6180607 in JvmtiEventControllerPrivate::set_event_callbacks(JvmtiEnvBase*, jvmtiEventCallbacks const*, int) () 
#3  0x00007ffff6181557 in JvmtiEventController::set_event_callbacks(JvmtiEnvBase*, jvmtiEventCallbacks const*, int) () 
#4  0x00007ffff6169ca2 in JvmtiEnv::SetEventCallbacks(jvmtiEventCallbacks const*, int) ()
#5  0x00007ffff61206d1 in jvmti_SetEventCallbacks ()
#6  0x00007ffff47bed27 in setLivePhaseEventHandlers ()
#7  0x00007ffff47be47f in processJavaStart ()
#8  0x00007ffff47bd0fe in eventHandlerVMInit ()
#9  0x00007ffff6183727 in JvmtiExport::post_vm_initialized() ()
#10 0x00007ffff65d560c in Threads::create_vm(JavaVMInitArgs*, bool*) ()
#11 0x00007ffff607fc6f in JNI_CreateJavaVM_inner(JavaVM_**, void**, void*) ()
#12 0x00007ffff607fffd in JNI_CreateJavaVM ()
#13 0x00007ffff7798c9e in InitializeJVM ()
#14 0x00007ffff77960ba in JavaMain ()
#15 0x00007ffff79ade9a in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#16 0x00007ffff72c34bd in clone () from /lib/x86_64-linux-gnu/libc.so.6
#17 0x0000000000000000 in ?? ()

If we don’t want to abort the VM running -Xshare:on with dynamically attached agent, maybe we can do the JvmtiExport::should_post_class_file_load_hook check after JvmtiExport::post_vm_initialized(). Then we can remove the 'if (RequireSharedSpaces) {‘ blocks from the load_shared_class().

Thanks,

Jiangli

> 
> 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
>> 
>> 
>