diff --git a/src/hotspot/share/classfile/placeholders.cpp b/src/hotspot/share/classfile/placeholders.cpp index f6614881e0d..ff9e34d274a 100644 --- a/src/hotspot/share/classfile/placeholders.cpp +++ b/src/hotspot/share/classfile/placeholders.cpp @@ -191,7 +191,11 @@ bool PlaceholderEntry::remove_seen_thread(JavaThread* thread, PlaceholderTable:: } -// Placeholder methods +void PlaceholderEntry::set_supername(Symbol* supername) { + assert_locked_or_safepoint(SystemDictionary_lock); + assert(_supername == nullptr || _supername->refcount() > 1, "must be referenced also by the loader"); + _supername = supername; +} // Placeholder objects represent classes currently being loaded. // All threads examining the placeholder table must hold the @@ -272,7 +276,6 @@ PlaceholderEntry* PlaceholderTable::find_and_add(Symbol* name, // placeholder is used to track class loading internal states -// placeholder existence now for loading superclass/superinterface // superthreadQ tracks class circularity, while loading superclass/superinterface // loadInstanceThreadQ tracks load_instance_class calls // definer() tracks the single thread that owns define token @@ -283,21 +286,21 @@ PlaceholderEntry* PlaceholderTable::find_and_add(Symbol* name, // On removal: if definer and all queues empty, remove entry // Note: you can be in both placeholders and systemDictionary // Therefore - must always check SD first -// Ignores the case where entry is not found void PlaceholderTable::find_and_remove(Symbol* name, ClassLoaderData* loader_data, classloadAction action, JavaThread* thread) { assert_locked_or_safepoint(SystemDictionary_lock); PlaceholderEntry* probe = get_entry(name, loader_data); - if (probe != nullptr) { - log(name, probe, "find_and_remove", action); - probe->remove_seen_thread(thread, action); - // If no other threads using this entry, and this thread is not using this entry for other states - if ((probe->superThreadQ() == nullptr) && (probe->loadInstanceThreadQ() == nullptr) - && (probe->defineThreadQ() == nullptr) && (probe->definer() == nullptr)) { - probe->clear_supername(); - remove_entry(name, loader_data); - } + assert(probe != nullptr, "must find an entry"); + log(name, probe, "find_and_remove", action); + probe->remove_seen_thread(thread, action); + if (probe->superThreadQ() == nullptr) { + probe->set_supername(nullptr); + } + // If no other threads using this entry, and this thread is not using this entry for other states + if ((probe->superThreadQ() == nullptr) && (probe->loadInstanceThreadQ() == nullptr) + && (probe->defineThreadQ() == nullptr) && (probe->definer() == nullptr)) { + remove_entry(name, loader_data); } } diff --git a/src/hotspot/share/classfile/placeholders.hpp b/src/hotspot/share/classfile/placeholders.hpp index d8f817da4f9..72235a689f1 100644 --- a/src/hotspot/share/classfile/placeholders.hpp +++ b/src/hotspot/share/classfile/placeholders.hpp @@ -25,7 +25,7 @@ #ifndef SHARE_CLASSFILE_PLACEHOLDERS_HPP #define SHARE_CLASSFILE_PLACEHOLDERS_HPP -#include "oops/symbol.hpp" +#include "oops/symbolHandle.hpp" class PlaceholderEntry; class Thread; @@ -81,7 +81,7 @@ class SeenThread; class PlaceholderEntry { friend class PlaceholderTable; private: - Symbol* _supername; + SymbolHandle _supername; JavaThread* _definer; // owner of define token InstanceKlass* _instanceKlass; // InstanceKlass from successful define SeenThread* _superThreadQ; // doubly-linked queue of Threads loading a superclass for this class @@ -99,30 +99,6 @@ class PlaceholderEntry { void add_seen_thread(JavaThread* thread, PlaceholderTable::classloadAction action); bool remove_seen_thread(JavaThread* thread, PlaceholderTable::classloadAction action); - public: - PlaceholderEntry() : - _supername(nullptr), _definer(nullptr), _instanceKlass(nullptr), - _superThreadQ(nullptr), _loadInstanceThreadQ(nullptr), _defineThreadQ(nullptr) { } - - Symbol* supername() const { return _supername; } - void set_supername(Symbol* supername) { - if (supername != _supername) { - Symbol::maybe_decrement_refcount(_supername); - _supername = supername; - Symbol::maybe_increment_refcount(_supername); - } - } - void clear_supername() { - Symbol::maybe_decrement_refcount(_supername); - _supername = nullptr; - } - - JavaThread* definer() const {return _definer; } - void set_definer(JavaThread* definer) { _definer = definer; } - - InstanceKlass* instance_klass() const {return _instanceKlass; } - void set_instance_klass(InstanceKlass* ik) { _instanceKlass = ik; } - SeenThread* superThreadQ() const { return _superThreadQ; } void set_superThreadQ(SeenThread* SeenThread) { _superThreadQ = SeenThread; } @@ -131,6 +107,19 @@ class PlaceholderEntry { SeenThread* defineThreadQ() const { return _defineThreadQ; } void set_defineThreadQ(SeenThread* SeenThread) { _defineThreadQ = SeenThread; } + public: + PlaceholderEntry() : + _definer(nullptr), _instanceKlass(nullptr), + _superThreadQ(nullptr), _loadInstanceThreadQ(nullptr), _defineThreadQ(nullptr) { } + + Symbol* supername() const { return _supername; } + void set_supername(Symbol* supername); + + JavaThread* definer() const {return _definer; } + void set_definer(JavaThread* definer) { _definer = definer; } + + InstanceKlass* instance_klass() const {return _instanceKlass; } + void set_instance_klass(InstanceKlass* ik) { _instanceKlass = ik; } bool super_load_in_progress() { return (_superThreadQ != nullptr); diff --git a/src/hotspot/share/oops/symbolHandle.hpp b/src/hotspot/share/oops/symbolHandle.hpp index f4388302310..249da936761 100644 --- a/src/hotspot/share/oops/symbolHandle.hpp +++ b/src/hotspot/share/oops/symbolHandle.hpp @@ -52,8 +52,7 @@ public: // Does not increment the current reference count if temporary. SymbolHandleBase(Symbol *s) : _temp(s) { if (!TEMP) { - assert(s != nullptr, "must not be null"); - s->increment_refcount(); + Symbol::maybe_increment_refcount(_temp); } } diff --git a/test/hotspot/gtest/classfile/test_placeholders.cpp b/test/hotspot/gtest/classfile/test_placeholders.cpp index 612482ba8b9..9fa06d49f73 100644 --- a/test/hotspot/gtest/classfile/test_placeholders.cpp +++ b/test/hotspot/gtest/classfile/test_placeholders.cpp @@ -63,14 +63,12 @@ TEST_VM(PlaceholderTable, supername) { PlaceholderTable::find_and_add(D, loader_data, super_action, interf, T2); PlaceholderTable::find_and_remove(D, loader_data, super_action, T2); - ASSERT_EQ(interf->refcount(), 3) << "supername isn't replaced until super set"; + ASSERT_EQ(interf->refcount(), 1) << "supername is replaced with null"; // Add placeholder to the table for loading A and super, and D also loading super PlaceholderTable::find_and_add(A, loader_data, super_action, super, THREAD); PlaceholderTable::find_and_add(D, loader_data, super_action, super, T2); - ASSERT_EQ(interf->refcount(), 1) << "now should be one"; - // Another thread comes in and finds A loading Super PlaceholderEntry* placeholder = PlaceholderTable::get_entry(A, loader_data); SymbolHandle supername = placeholder->supername();