diff --git a/src/hotspot/share/classfile/placeholders.cpp b/src/hotspot/share/classfile/placeholders.cpp index f6614881e0da3..ff9e34d274afe 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 d8f817da4f92f..72235a689f15b 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,23 +99,21 @@ class PlaceholderEntry { void add_seen_thread(JavaThread* thread, PlaceholderTable::classloadAction action); bool remove_seen_thread(JavaThread* thread, PlaceholderTable::classloadAction action); + SeenThread* superThreadQ() const { return _superThreadQ; } + void set_superThreadQ(SeenThread* SeenThread) { _superThreadQ = SeenThread; } + + SeenThread* loadInstanceThreadQ() const { return _loadInstanceThreadQ; } + void set_loadInstanceThreadQ(SeenThread* SeenThread) { _loadInstanceThreadQ = SeenThread; } + + SeenThread* defineThreadQ() const { return _defineThreadQ; } + void set_defineThreadQ(SeenThread* SeenThread) { _defineThreadQ = SeenThread; } public: PlaceholderEntry() : - _supername(nullptr), _definer(nullptr), _instanceKlass(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; - } + void set_supername(Symbol* supername); JavaThread* definer() const {return _definer; } void set_definer(JavaThread* definer) { _definer = definer; } @@ -123,15 +121,6 @@ class PlaceholderEntry { 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; } - - SeenThread* loadInstanceThreadQ() const { return _loadInstanceThreadQ; } - void set_loadInstanceThreadQ(SeenThread* SeenThread) { _loadInstanceThreadQ = SeenThread; } - - SeenThread* defineThreadQ() const { return _defineThreadQ; } - void set_defineThreadQ(SeenThread* SeenThread) { _defineThreadQ = SeenThread; } - 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 f4388302310ac..249da936761ec 100644 --- a/src/hotspot/share/oops/symbolHandle.hpp +++ b/src/hotspot/share/oops/symbolHandle.hpp @@ -52,8 +52,7 @@ class SymbolHandleBase : public StackObj { // 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 612482ba8b9b8..9fa06d49f7306 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();