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

Convert an assert in ClassLoaderData to a guarantee

    Details

    • Subcomponent:
    • Resolved In Build:
      b120

      Backports

        Description

        In the following code, I suggest to change the assert to a guarantee.

        inline ClassLoaderData *ClassLoaderDataGraph::find_or_create(Handle loader, TRAPS) {
          assert(loader() != NULL,"Must be a class loader");
          // Gets the class loader data out of the java/lang/ClassLoader object, if non-null
          // it's already in the loader_data, so no need to add
          ClassLoaderData* loader_data= java_lang_ClassLoader::loader_data(loader());
          if (loader_data) {
             return loader_data;
          }
          return ClassLoaderDataGraph::add(loader, false, THREAD);
        }

        This guarantee should also check that the classloader is an oop. It should be something like this:
        guarantee(loader() != NULL && loader()->is_oop(), "loader must be oop");

        We struggled for a long time with an internal stress bug (which could not be run with the fastdebug build) where the JVM was crashing during GC because the object it was looking at had a pointer to a bad klass. The core file analysis showed that the classloader object in the klass was not a valid Oop and we suspected that the classloader object that the JVM received from Java land during class initialization was not a valid object. Unfortunately the problem stopped reproducing and we could not try a run with the above assert changed to guarantee.

        I think having a guarantee here ensuring that the classloader is a valid oop will help catch the errors at an early stage during the run rather than crashing the JVM later on in the GC.


          Activity

          Hide
          coleenp Coleen Phillimore added a comment -
          I'm trying to understand the RFR thread and why this change would help find the bug that you were looking for. The bug was that the CLD was unloaded while there were still uses of the Metadata (Klass*) which I don't know how it was tracked down.
          If the class_loader oop was unloaded and subsequently the CLD, then it's possible that this would be an entry point for the bad class_loader oop to come back into the JVM. So maybe this guarantee makes sense.
          Show
          coleenp Coleen Phillimore added a comment - I'm trying to understand the RFR thread and why this change would help find the bug that you were looking for. The bug was that the CLD was unloaded while there were still uses of the Metadata (Klass*) which I don't know how it was tracked down. If the class_loader oop was unloaded and subsequently the CLD, then it's possible that this would be an entry point for the bad class_loader oop to come back into the JVM. So maybe this guarantee makes sense.
          Hide
          hgupdate HG Updates added a comment -
          URL: http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/8341dddb5188
          User: poonam
          Date: 2016-04-29 15:59:55 +0000
          Show
          hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/8341dddb5188 User: poonam Date: 2016-04-29 15:59:55 +0000
          Hide
          hgupdate HG Updates added a comment -
          URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/8341dddb5188
          User: lana
          Date: 2016-05-25 17:36:48 +0000
          Show
          hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/8341dddb5188 User: lana Date: 2016-05-25 17:36:48 +0000

            People

            • Assignee:
              shshahma Shafi Ahmad
              Reporter:
              poonam Poonam Bajaj Parhar
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Due:
                Created:
                Updated:
                Resolved: