CDS and JVM-TI agent questions and crash

[Volker Simonis]

Just a quick analysis (will look into a fix tomorrow).

- When saving a class to the CDS archive, the VM calls
ConstantPool::remove_unshareable_info() on its constant pool:

void ConstantPool::remove_unshareable_info() {
...
  // Shared ConstantPools are in the RO region, so the _flags cannot
be modified.
  // The _on_stack flag is used to prevent ConstantPools from
deallocation during
  // class redefinition. Since shared ConstantPools cannot be
deallocated anyway,
  // we always set _on_stack to true to avoid having to change _flags
during runtime.
  _flags |= (_on_stack | _is_shared);

This calls sets the '_on_stack' flag on the constant pool for the
reasons described in the comment.

- When a class is being loaded from the CDS archive, the VM calls
ConstantPool::restore_unshareable_info(TRAPS) on its constant pool:

// CDS support. Create a new resolved_references array.
void ConstantPool::restore_unshareable_info(TRAPS) {
  assert(is_constantPool(), "ensure C++ vtable is restored");
  assert(on_stack(), "should always be set for shared constant pools");
  assert(is_shared(), "should always be set for shared constant pools");
  assert(_cache != NULL, "constant pool _cache should not be NULL");

This calls asserts for the '_on_stack' flag being set.

- When loading a class from the CSD archive
withSystemDictionary::load_shared_class(),
ConstantPool::restore_unshareable_info(TRAPS) is actually called from
InstanceKlass::restore_unshareable_info(TRAPS) after the instanceKlass
was loaded:

InstanceKlass* SystemDictionary::load_shared_class(InstanceKlass* ik,
                                                   Handle class_loader,
                                                   Handle
protection_domain, TRAPS) {
...
    InstanceKlass* new_ik = KlassFactory::check_shared_class_file_load_hook(
        ik, class_name, class_loader, protection_domain, CHECK_NULL);
    if (new_ik != NULL) {
      // The class is changed by CFLH. Return the new class. The shared class is
      // not used.
      return new_ik;
    }

    // Adjust methods to recover missing data.  They need addresses for
    // interpreter entry points and their default native method address
    // must be reset.

    // Updating methods must be done under a lock so multiple
    // threads don't update these in parallel
    //
    // Shared classes are all currently loaded by either the bootstrap or
    // internal parallel class loaders, so this will never cause a deadlock
    // on a custom class loader lock.

    ClassLoaderData* loader_data =
ClassLoaderData::class_loader_data(class_loader());
    {
      HandleMark hm(THREAD);
      Handle lockObject = compute_loader_lock_object(class_loader, THREAD);
      check_loader_lock_contention(lockObject, THREAD);
      ObjectLocker ol(lockObject, THREAD, true);
      // prohibited package check assumes all classes loaded from archive call
      // restore_unshareable_info which calls ik->set_package()
      ik->restore_unshareable_info(loader_data, protection_domain, CHECK_NULL);
    }

- However, when a class loaded from the CDS archive is being redefined
there's a shortcut in SystemDictionary::load_shared_class() which
instantly returns the new, redefined class, without calling
InstanceKlass::restore_unshareable_info() on it:

    InstanceKlass* new_ik = KlassFactory::check_shared_class_file_load_hook(
        ik, class_name, class_loader, protection_domain, CHECK_NULL);
    if (new_ik != NULL) {
      // The class is changed by CFLH. Return the new class. The shared class is
      // not used.
      return new_ik;
    }

- For the well known class java.lang.Object the procedure is a little
different. It gets loaded early in
SystemDictionary::resolve_preloaded_classes() and for some reason we
directly call ConstantPool::restore_unshareable_info() right on its
constant pool after it was loaded:

#if INCLUDE_CDS
  if (UseSharedSpaces) {
    resolve_wk_klasses_through(WK_KLASS_ENUM_NAME(Object_klass), scan, CHECK);
...
    // Initialize the constant pool for the Object_class
    Object_klass()->constants()->restore_unshareable_info(CHECK);

- This fails if java.lang.Object was redefined, because in that case
its constant pool won't have its '_on_stack' flag set to 'true'.

- I don't think that this extra call to
Object_klass()->constants()->restore_unshareable_info() is required
any more, because the call to
resolve_wk_klasses_through(WK_KLASS_ENUM_NAME(Object_klass)) already
loads java.lang.Object through SystemDictionary::load_shared_class()
which will call restore_unshareable_info() on it (if it was not
redefined). But I'll have to check this more thoroughly...

I'll do that and open a bug for this issue tomorrow.

Regards,
Volker

On Mon, Oct 15, 2018 at 5:32 PM Michael Rasmussen
<Michael.Rasmussen at roguewave.com> wrote:
>
> Hi
>
> I have some questions regarding Class Data Sharing and class file transforming agents:
>
> 1. Is it possible to detect during Agent_OnLoad if CDS is used or not?
> Checking for "sharing" in the "java.vm.info" system property, which seems to be how the tests are detecting it, is not an option, as this property contains bogus information during Agent_OnLoad.
>
> 2. Should the registration of a ClassFileHook, especially with can_generate_all_class_hook_events and can_generate_early_class_hook_events capabilities set, disable CDS?
>
> 3. Is it possible to explicitly disable CDS from a JVM-TI agent during Agent_OnLoad?
>
>
> The reason for these questions is that we (JRebel) recently became aware of an issue if CDS is used, where transforming some of the boot classes causes the JVM to crash.
> The simplest example of this is replacing Object's byte in a ClassFileLoadHook with a copy of themselves, see inlined example agent code below.
> I don't know if there are more issues than that, as that's where we are currently stuck.
>
> Running on a 11+28 fastdebug (with classes.jsa file present) it produces the following assert:
> #  Internal Error (.../src/hotspot/share/oops/constantPool.cpp:325), pid=589, tid=590
> #  assert(on_stack()) failed: should always be set for shared constant pools
>
> if running with -Xshare:off (or no classes.jsa file), it works without any issue.
>
> Kind regards
> Michael Rasmussen
>
> /* --- snip --- */
> #include <string.h>
> #include <jni.h>
> #include <jvmti.h>
>
> void JNICALL callback_ClassFileLoadHook(jvmtiEnv *jvmti, JNIEnv* env,
>         jclass class_being_redefined, jobject loader, const char* name, jobject protection_domain,
>         jint class_data_len, const unsigned char* class_data,
>         jint* new_class_data_len, unsigned char** new_class_data) {
>
>         if (strcmp("java/lang/Object", name) == 0) {
>                 *new_class_data_len = class_data_len;
>                 (*jvmti)->Allocate(jvmti, *new_class_data_len, new_class_data);
>                 memcpy(*new_class_data, class_data, *new_class_data_len);
>         }
> }
>
> JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM *vm, char *options, void *reserved) {
>         jvmtiEnv *jvmti = NULL;
>         jint rc = (*vm)->GetEnv(vm, (void **)&jvmti, JVMTI_VERSION_9);
>
>         jvmtiCapabilities caps;
>         memset(&caps, 0, sizeof(caps));
>
>         caps.can_redefine_classes = 1;
>         caps.can_generate_all_class_hook_events = 1;
>         caps.can_generate_early_vmstart = 1;
>         caps.can_generate_early_class_hook_events = 1;
>
>         (*jvmti)->AddCapabilities(jvmti, &caps);
>
>         jvmtiEventCallbacks callbacks;
>         memset(&callbacks, 0, sizeof(callbacks));
>         callbacks.ClassFileLoadHook = &callback_ClassFileLoadHook;
>         (*jvmti)->SetEventCallbacks(jvmti, &callbacks, sizeof(callbacks));
>         (*jvmti)->SetEventNotificationMode(jvmti, JVMTI_ENABLE, JVMTI_EVENT_CLASS_FILE_LOAD_HOOK, NULL);
>
>         return 0;
> }
> /* --- snap --- */