Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-6875393

JNI itable index cache is broken

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: P2 P2
    • hs17
    • hs16, 6
    • hotspot
    • b83
    • 6
    • b02
    • generic
    • generic
    • Verified

        The JNI itable index cache is managed by the following functions
        in src/share/vm/oops/instanceKlass.cpp:

        instanceKlass::set_cached_itable_index(size_t idnum, int index)
        instanceKlass::cached_itable_index(size_t idnum)

        These functions were added by the fix for the following bug:

            6404550 1/1 Cannot implement late attach in NetBeans Profiler
                        due to missing functionality in JVMTI

        The fix for 6404550 was integrated in Mustang-B83.

        The cache is used in

        src/share/vm/prims/jni.cpp: jni_invoke_nonstatic()

            } else {
              // interface call
              KlassHandle h_holder(THREAD, holder);

              int itbl_index = m->cached_itable_index();
              if (itbl_index == -1) {
                itbl_index = klassItable::compute_itable_index(m);
                m->set_cached_itable_index(itbl_index);
                // the above may have grabbed a lock, 'm' and anything non-handlized can't be used again
              }
              klassOop k = h_recv->klass();
              selected_method = instanceKlass::cast(k)->method_at_itable(h_holder(), itbl_index, CHECK);
            }

        Figuring out a method's itable index for a JNI invoke is expensive
        so the idea is to cache the itable index on the first JNI invoke
        of the method and then reuse it for subsequent invokes.

        However, while I was investigating the following bug:

            6419370 4/4 new jmethodID code has tiny holes in synchronization

        I noticed that set_cached_itable_index() was allocating a new,
        full size cache array for every cache access attempt. On further
        investigation, I figured out that the newly allocated cache was
        missing the critical assignment to element zero:

            // array length stored in first element, other elements offset by one
            if (indices == NULL || (length = (size_t)indices[0]) <= idnum) {
              size_t size = MAX2(idnum+1, (size_t)idnum_allocated_count());
              int* new_indices = NEW_C_HEAP_ARRAY(int, size+1);
        /*MISSING*/ new_indices[0] =(int)size; // array size held in the first element
              // Copy the existing entries, if any
              size_t i;
              for (i = 0; i < length; i++) {
                new_indices[i+1] = indices[i+1];
              }
              // Set all the rest to -1
              for (i = length; i < size; i++) {
                new_indices[i+1] = -1;
              }
              if (indices != NULL) {
                FreeHeap(indices); // delete any old indices
              }
              release_set_methods_cached_itable_indices(indices = new_indices);

        The interesting part of this bug is that the missing
        setting of the cache size means that we allocate a new
        cache and free the old cache on every cache access. A
        really stressful situation that has helped me reproduce
        one of the races described in 6419370.

              dcubed Daniel Daugherty
              dcubed Daniel Daugherty
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

                Created:
                Updated:
                Resolved:
                Imported:
                Indexed: