From b3d75fe12ec74e3c2445ef2615425867ccb7d4a2 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Thu, 21 Sep 2023 12:17:34 +0000 Subject: [PATCH] 8310874: Runthese30m crashes with klass should be in the placeholders during verification Reviewed-by: dholmes, iklam --- .../share/classfile/loaderConstraints.cpp | 32 +++++++++++-------- .../share/classfile/loaderConstraints.hpp | 3 +- .../share/classfile/systemDictionary.cpp | 4 +++ 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/hotspot/share/classfile/loaderConstraints.cpp b/src/hotspot/share/classfile/loaderConstraints.cpp index c447914b852..a613bb9f26d 100644 --- a/src/hotspot/share/classfile/loaderConstraints.cpp +++ b/src/hotspot/share/classfile/loaderConstraints.cpp @@ -27,15 +27,11 @@ #include "classfile/classLoaderDataGraph.hpp" #include "classfile/dictionary.hpp" #include "classfile/loaderConstraints.hpp" -#include "classfile/placeholders.hpp" #include "logging/log.hpp" #include "memory/resourceArea.hpp" #include "oops/klass.inline.hpp" -#include "oops/oop.inline.hpp" #include "oops/symbolHandle.hpp" -#include "runtime/handles.inline.hpp" #include "runtime/mutexLocker.hpp" -#include "runtime/safepoint.hpp" #include "utilities/resourceHash.hpp" // Overview @@ -447,6 +443,23 @@ InstanceKlass* LoaderConstraintTable::find_constrained_klass(Symbol* name, return nullptr; } +// Removes a class that was added to the table then class loading subsequently failed for this class, +// so we don't have a dangling pointer to InstanceKlass in the LoaderConstraintTable. +void LoaderConstraintTable::remove_failed_loaded_klass(InstanceKlass* klass, + ClassLoaderData* loader) { + + Symbol* name = klass->name(); + LoaderConstraint *p = find_loader_constraint(name, loader); + if (p != nullptr && p->klass() != nullptr && p->klass() == klass) { + // If this is the klass in the constraint, the error was OOM from the ClassLoader.addClass() call. + // Other errors during loading (eg. constraint violations) will not have added this klass. + log_info(class, loader, constraints)("removing klass %s: failed to load", name->as_C_string()); + // We only null out the class, since the constraint for the class name for this loader is still valid as + // it was added when checking signature loaders for a method or field resolution. + p->set_klass(nullptr); + } +} + void LoaderConstraintTable::merge_loader_constraints(Symbol* class_name, LoaderConstraint* p1, LoaderConstraint* p2, @@ -511,15 +524,8 @@ void LoaderConstraintTable::verify() { // We found the class in the dictionary, so we should // make sure that the Klass* matches what we already have. guarantee(k == probe->klass(), "klass should be in dictionary"); - } else { - // If we don't find the class in the dictionary, it - // has to be in the placeholders table. - PlaceholderEntry* entry = PlaceholderTable::get_entry(name, loader_data); - - // The InstanceKlass might not be on the entry, so the only - // thing we can check here is whether we were successful in - // finding the class in the placeholders table. - guarantee(entry != nullptr, "klass should be in the placeholders"); + // If we don't find the class in the dictionary, it is + // in the process of loading and may or may not be in the placeholder table. } } for (int n = 0; n< probe->num_loaders(); n++) { diff --git a/src/hotspot/share/classfile/loaderConstraints.hpp b/src/hotspot/share/classfile/loaderConstraints.hpp index bdf8fac51fa..8153d9c28fd 100644 --- a/src/hotspot/share/classfile/loaderConstraints.hpp +++ b/src/hotspot/share/classfile/loaderConstraints.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -59,6 +59,7 @@ public: // Class loader constraints static bool check_or_update(InstanceKlass* k, ClassLoaderData* loader, Symbol* name); + static void remove_failed_loaded_klass(InstanceKlass* k, ClassLoaderData* loader); static void purge_loader_constraints(); static void print_table_statistics(outputStream* st); diff --git a/src/hotspot/share/classfile/systemDictionary.cpp b/src/hotspot/share/classfile/systemDictionary.cpp index d9da1a4e843..10ceb008794 100644 --- a/src/hotspot/share/classfile/systemDictionary.cpp +++ b/src/hotspot/share/classfile/systemDictionary.cpp @@ -1526,6 +1526,10 @@ InstanceKlass* SystemDictionary::find_or_define_instance_class(Symbol* class_nam } else if (HAS_PENDING_EXCEPTION) { assert(defined_k == nullptr, "Should not have a klass if there's an exception"); k->class_loader_data()->add_to_deallocate_list(k); + + // Also remove this InstanceKlass from the LoaderConstraintTable if added. + MutexLocker ml(SystemDictionary_lock); + LoaderConstraintTable::remove_failed_loaded_klass(k, class_loader_data(class_loader)); } return defined_k; }