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

Unify ResourceHashtable APIs for removing entries

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Unresolved
    • Icon: P3 P3
    • tbd
    • 21
    • hotspot

      When an entry is removed from a ResourceHashtable, sometimes it's necessary to perform special cleanup. Currently this is only possible via the unlink() API:

      https://github.com/openjdk/jdk/blob/5dfc4ec7d94af9fe39fdee9d83b06101b827a3c6/src/hotspot/share/utilities/resourceHash.hpp#L258-L278

        template<class ITER>
        void unlink(ITER* iter, bool dummy) {
          const unsigned sz = table_size();
          for (unsigned index = 0; index < sz; index++) {
            Node** ptr = bucket_at(index);
            while (*ptr != nullptr) {
              Node* node = *ptr;
              // do_entry must clean up the key and value in Node.
              bool clean = iter->do_entry(node->_key, node->_value);
              if (clean) {
                *ptr = node->_next;
                if (ALLOC_TYPE == AnyObj::C_HEAP) {
                  delete node;
                }

      If the node->_value needs special clean up, it must be done inside iter->do_entry(). Here's an example:

      https://github.com/openjdk/jdk/blob/5dfc4ec7d94af9fe39fdee9d83b06101b827a3c6/src/hotspot/share/oops/instanceKlass.cpp#L1005-L1020

      void InstanceKlass::clean_initialization_error_table() {
        struct InitErrorTableCleaner {
          bool do_entry(const InstanceKlass* ik, OopHandle h) {
            if (!ik->is_loader_alive()) {
              h.release(Universe::vm_global());
              return true;
            } else {
              return false;
            }
          }
        };

        assert_locked_or_safepoint(ClassInitError_lock);
        InitErrorTableCleaner cleaner;
        _initialization_error_table.unlink(&cleaner);
      }

      However, entries can also be removed via the remove() API, or in the ResourceHashtable's destructor. In these cases, the caller must perform the special clean up in a different way than with the unlink() API.

      With the current implementation, if the entries require special clean up, we really should assert in the ResourceHashtable's destructor that the table must be empty, or else there would be memory leak. However, with the current API, there is no way to declare that the entries need special clean up.

      Also, the destructors for the _key and _value members of the ResourceHashtableNode are NOT called if the table is allocated using AnyObj::RESOURCE_AREA. This will lead to memory leaks if the table contains SymbolHandle, for example.

      ======================================
      Proposal:

      [1] The ResourceHashtableNode destructor should be called even if the table is allocated in RESOURCE_AREA. This ensures that the ~K() and ~V() destructors are called.

      [2] Look at all the calls to the unlink() APIs -- for most of them, we should be able to perform the clean up inside the ~K() and ~V() destructors. Example: SourceObjInfo. See JDK-8301304.

      [3] unlink(Function F) should be changed so that F cannot modify the key or the value. I.e., F should be used only to check whether an entry should be removed or not. It should not be used for deallocation.

      [4] See if we still need a special variant of unlink(Function F) that allows F to modify the value. E.g., can the InstanceKlass::clean_initialization_error_table() example above be implemented using destructors?


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

              Created:
              Updated: