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

Rename DictionaryEntry members related to protection domain

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Fixed
    • Icon: P4 P4
    • 23
    • None
    • hotspot
    • b15

      The fields and methods of DictionaryEntry have non-intuitive names, so much so that we need a comment that effectively says, "they are not what you think they are"

      https://github.com/openjdk/jdk/blob/de428daf9adef5afe7347319f7a6f6732e9b6c4b/src/hotspot/share/classfile/dictionary.hpp#L101-L117

        // Contains the set of approved protection domains that can access
        // this dictionary entry.
        //
        // [Note that C.protection_domain(), which is stored in the java.lang.Class
        // mirror of C, is NOT the same as PD]
        //
        // If an entry for PD exists in the list, it means that
        // it is okay for a caller class to reference the class in this dictionary entry.
        //
        // The usage of the PD set can be seen in SystemDictionary::validate_protection_domain()
        // It is essentially a cache to avoid repeated Java up-calls to
        // ClassLoader.checkPackageAccess().
        //
        InstanceKlass* _instance_klass;
        ProtectionDomainEntry* volatile _pd_set;

      Other function names are problematic:

      DictionaryEntry::is_valid_protection_domain() - what does "valid" mean? Also, if this function return false, it would seem that it means "this PD is invalid", but the real meaning is "this PD has not yet been painstakingly validated, but it might actually be valid".

      DictionaryEntry::add_protection_domain()/contains_protection_domain() - what is the effect of "adding a PD" do to a DictionaryEntry

      =========================================
      Proposal -- DictionaryEntry::_pd_set and the related functions are used solely for calling java.lang.ClassLoader.checkPackageAccess(class, pd). We should rename the fields and methods in DictionaryEntry to reflect this.

            iklam Ioi Lam
            iklam Ioi Lam
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: