Skip to content

Commit c6be2cd

Browse files
tstuefetstuefe
committed
8293156: Dcmd VM.classloaders fails to print the full hierarchy
Reviewed-by: dholmes, cjplummer
1 parent 711e252 commit c6be2cd

File tree

2 files changed

+136
-54
lines changed

2 files changed

+136
-54
lines changed

‎src/hotspot/share/classfile/classLoaderHierarchyDCmd.cpp‎

Lines changed: 65 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ struct LoadedClassInfo : public ResourceObj {
122122
const ClassLoaderData* const _cld;
123123

124124
LoadedClassInfo(Klass* klass, const ClassLoaderData* cld)
125-
: _klass(klass), _cld(cld) {}
125+
: _next(NULL), _klass(klass), _cld(cld) {}
126126

127127
};
128128

@@ -137,7 +137,7 @@ class LoaderTreeNode : public ResourceObj {
137137
// this parent loader, we fill in all the other details.
138138

139139
const oop _loader_oop;
140-
const ClassLoaderData* _cld;
140+
const ClassLoaderData* _cld; // May be NULL if loader never loaded anything
141141

142142
LoaderTreeNode* _child;
143143
LoaderTreeNode* _next;
@@ -154,33 +154,59 @@ class LoaderTreeNode : public ResourceObj {
154154
// one.
155155
int _num_folded;
156156

157-
void print_with_childs(outputStream* st, BranchTracker& branchtracker,
158-
bool print_classes, bool verbose) const {
157+
// Returns Klass of loader; NULL for bootstrap loader
158+
const Klass* loader_klass() const {
159+
return (_loader_oop != NULL) ? _loader_oop->klass() : NULL;
160+
}
159161

160-
ResourceMark rm;
162+
// Returns ResourceArea-allocated class name of loader class; "" if there is no klass (bootstrap loader)
163+
const char* loader_class_name() const {
164+
const Klass* klass = loader_klass();
165+
return klass != NULL ? klass->external_name() : "";
166+
}
161167

162-
if (_cld == NULL) {
163-
// Not sure how this could happen: we added a preliminary node for a parent but then never encountered
164-
// its CLD?
165-
return;
168+
// Returns oop of loader name; NULL for bootstrap; NULL if no name was set
169+
oop loader_name_oop() const {
170+
return (_loader_oop != NULL) ? java_lang_ClassLoader::name(_loader_oop) : NULL;
171+
}
172+
173+
// Returns ResourceArea-allocated name of loader, "" if none is set
174+
const char* loader_name() const {
175+
oop name_oop = loader_name_oop();
176+
return name_oop != NULL ? java_lang_String::as_utf8_string(name_oop) : "";
177+
}
178+
179+
bool is_bootstrap() const {
180+
if (_loader_oop == NULL) {
181+
assert(_cld != NULL && _cld->is_boot_class_loader_data(), "bootstrap loader must have CLD");
182+
return true;
166183
}
184+
return false;
185+
}
186+
187+
void print_with_child_nodes(outputStream* st, BranchTracker& branchtracker,
188+
bool print_classes, bool verbose) const {
189+
190+
assert(SafepointSynchronize::is_at_safepoint(), "invariant");
191+
192+
ResourceMark rm;
167193

168194
// Retrieve information.
169-
const Klass* const loader_klass = _cld->class_loader_klass();
170-
const Symbol* const loader_name = _cld->name();
195+
const Klass* const the_loader_klass = loader_klass();
196+
const char* const the_loader_class_name = loader_class_name();
197+
const char* const the_loader_name = loader_name();
171198

172199
branchtracker.print(st);
173200

174201
// e.g. +-- "app", jdk.internal.loader.ClassLoaders$AppClassLoader
175202
st->print("+%.*s", BranchTracker::twig_len, "----------");
176-
if (_cld->is_the_null_class_loader_data()) {
203+
if (is_bootstrap()) {
177204
st->print(" <bootstrap>");
178205
} else {
179-
assert(!_cld->has_class_mirror_holder(), "_cld must be the primary cld");
180-
if (loader_name != NULL) {
181-
st->print(" \"%s\",", loader_name->as_C_string());
206+
if (the_loader_name[0] != '\0') {
207+
st->print(" \"%s\",", the_loader_name);
182208
}
183-
st->print(" %s", loader_klass != NULL ? loader_klass->external_name() : "??");
209+
st->print(" %s", the_loader_class_name);
184210
if (_num_folded > 0) {
185211
st->print(" (+ %d more)", _num_folded);
186212
}
@@ -210,7 +236,7 @@ class LoaderTreeNode : public ResourceObj {
210236
branchtracker.print(st);
211237
st->print_cr("%*s " PTR_FORMAT, indentation, "Loader Data:", p2i(_cld));
212238
branchtracker.print(st);
213-
st->print_cr("%*s " PTR_FORMAT, indentation, "Loader Klass:", p2i(loader_klass));
239+
st->print_cr("%*s " PTR_FORMAT, indentation, "Loader Klass:", p2i(the_loader_klass));
214240

215241
// Empty line
216242
branchtracker.print(st);
@@ -219,6 +245,7 @@ class LoaderTreeNode : public ResourceObj {
219245

220246
if (print_classes) {
221247
if (_classes != NULL) {
248+
assert(_cld != NULL, "we have classes, we should have a CLD");
222249
for (LoadedClassInfo* lci = _classes; lci; lci = lci->_next) {
223250
// non-strong hidden classes should not live in
224251
// the primary CLD of their loaders.
@@ -243,6 +270,7 @@ class LoaderTreeNode : public ResourceObj {
243270
}
244271

245272
if (_hidden_classes != NULL) {
273+
assert(_cld != NULL, "we have classes, we should have a CLD");
246274
for (LoadedClassInfo* lci = _hidden_classes; lci; lci = lci->_next) {
247275
branchtracker.print(st);
248276
if (lci == _hidden_classes) { // first iteration
@@ -276,7 +304,7 @@ class LoaderTreeNode : public ResourceObj {
276304
// Print children, recursively
277305
LoaderTreeNode* c = _child;
278306
while (c != NULL) {
279-
c->print_with_childs(st, branchtracker, print_classes, verbose);
307+
c->print_with_child_nodes(st, branchtracker, print_classes, verbose);
280308
c = c->_next;
281309
}
282310

@@ -285,10 +313,21 @@ class LoaderTreeNode : public ResourceObj {
285313
// Helper: Attempt to fold this node into the target node. If success, returns true.
286314
// Folding can be done if both nodes are leaf nodes and they refer to the same loader class
287315
// and they have the same name or no name (note: leaf check is done by caller).
288-
bool can_fold_into(LoaderTreeNode* target_node) const {
316+
bool can_fold_into(const LoaderTreeNode* target_node) const {
289317
assert(is_leaf() && target_node->is_leaf(), "must be leaf");
290-
return _cld->class_loader_klass() == target_node->_cld->class_loader_klass() &&
291-
_cld->name() == target_node->_cld->name();
318+
319+
// Must have the same non-null klass
320+
const Klass* k = loader_klass();
321+
if (k == NULL || k != target_node->loader_klass()) {
322+
return false;
323+
}
324+
325+
// Must have the same loader name, or none
326+
if (::strcmp(loader_name(), target_node->loader_name()) != 0) {
327+
return false;
328+
}
329+
330+
return true;
292331
}
293332

294333
public:
@@ -300,6 +339,7 @@ class LoaderTreeNode : public ResourceObj {
300339
{}
301340

302341
void set_cld(const ClassLoaderData* cld) {
342+
assert(_cld == NULL, "there should be only one primary CLD per loader");
303343
_cld = cld;
304344
}
305345

@@ -334,14 +374,6 @@ class LoaderTreeNode : public ResourceObj {
334374
}
335375
}
336376

337-
const ClassLoaderData* cld() const {
338-
return _cld;
339-
}
340-
341-
const oop loader_oop() const {
342-
return _loader_oop;
343-
}
344-
345377
LoaderTreeNode* find(const oop loader_oop) {
346378
LoaderTreeNode* result = NULL;
347379
if (_loader_oop == loader_oop) {
@@ -364,6 +396,7 @@ class LoaderTreeNode : public ResourceObj {
364396
void fold_children() {
365397
LoaderTreeNode* node = _child;
366398
LoaderTreeNode* prev = NULL;
399+
ResourceMark rm;
367400
while (node != NULL) {
368401
LoaderTreeNode* matching_node = NULL;
369402
if (node->is_leaf()) {
@@ -389,9 +422,9 @@ class LoaderTreeNode : public ResourceObj {
389422
}
390423
}
391424

392-
void print_with_childs(outputStream* st, bool print_classes, bool print_add_info) const {
425+
void print_with_child_nodes(outputStream* st, bool print_classes, bool print_add_info) const {
393426
BranchTracker bwt;
394-
print_with_childs(st, bwt, print_classes, print_add_info);
427+
print_with_child_nodes(st, bwt, print_classes, print_add_info);
395428
}
396429

397430
};
@@ -466,7 +499,7 @@ class LoaderInfoScanClosure : public CLDClosure {
466499
}
467500

468501
void print_results(outputStream* st) const {
469-
_root->print_with_childs(st, _print_classes, _verbose);
502+
_root->print_with_child_nodes(st, _print_classes, _verbose);
470503
}
471504

472505
void do_cld (ClassLoaderData* cld) {
@@ -483,7 +516,6 @@ class LoaderInfoScanClosure : public CLDClosure {
483516

484517
// Update CLD in node, but only if this is the primary CLD for this loader.
485518
if (cld->has_class_mirror_holder() == false) {
486-
assert(info->cld() == NULL, "there should be only one primary CLD per loader");
487519
info->set_cld(cld);
488520
}
489521

‎test/hotspot/jtreg/serviceability/dcmd/vm/ClassLoaderHierarchyTest.java‎

Lines changed: 71 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,19 @@
4848

4949
public class ClassLoaderHierarchyTest {
5050

51+
class EmptyDelegatingLoader extends ClassLoader {
52+
EmptyDelegatingLoader(String name, ClassLoader parent) {
53+
super(name, parent);
54+
}
55+
}
56+
57+
static void loadTestClassInLoaderAndCheck(String classname, ClassLoader loader) throws ClassNotFoundException {
58+
Class<?> c = Class.forName(classname, true, loader);
59+
if (c.getClassLoader() != loader) {
60+
Assert.fail(classname + " defined by wrong classloader: " + c.getClassLoader());
61+
}
62+
}
63+
5164
//+-- <bootstrap>
5265
// |
5366
// +-- "platform", jdk.internal.loader.ClassLoaders$PlatformClassLoader
@@ -60,39 +73,76 @@ public class ClassLoaderHierarchyTest {
6073

6174
public void run(CommandExecutor executor) throws ClassNotFoundException {
6275

76+
// A) one unnamed, two named loaders
6377
ClassLoader unnamed_cl = new TestClassLoader(null, null);
64-
Class<?> c1 = Class.forName("TestClass2", true, unnamed_cl);
65-
if (c1.getClassLoader() != unnamed_cl) {
66-
Assert.fail("TestClass defined by wrong classloader: " + c1.getClassLoader());
67-
}
68-
6978
ClassLoader named_cl = new TestClassLoader("Kevin", null);
70-
Class<?> c2 = Class.forName("TestClass2", true, named_cl);
71-
if (c2.getClassLoader() != named_cl) {
72-
Assert.fail("TestClass defined by wrong classloader: " + c2.getClassLoader());
73-
}
74-
7579
ClassLoader named_child_cl = new TestClassLoader("Bill", unnamed_cl);
76-
Class<?> c3 = Class.forName("TestClass2", true, named_child_cl);
77-
if (c3.getClassLoader() != named_child_cl) {
78-
Assert.fail("TestClass defined by wrong classloader: " + c3.getClassLoader());
79-
}
80+
loadTestClassInLoaderAndCheck("TestClass2", unnamed_cl);
81+
loadTestClassInLoaderAndCheck("TestClass2", named_cl);
82+
loadTestClassInLoaderAndCheck("TestClass2", named_child_cl);
83+
84+
// B) A named CL with empty loaders as parents (JDK-8293156)
85+
EmptyDelegatingLoader emptyLoader1 = new EmptyDelegatingLoader("EmptyLoader1", null);
86+
EmptyDelegatingLoader emptyLoader2 = new EmptyDelegatingLoader("EmptyLoader2", emptyLoader1);
87+
ClassLoader named_child_2_cl = new TestClassLoader("Child2", emptyLoader2);
88+
loadTestClassInLoaderAndCheck("TestClass2", named_child_2_cl);
89+
90+
// C) Test output for several class loaders, same class, same name, empty parents,
91+
// and all these should be folded by default.
92+
EmptyDelegatingLoader emptyLoader3 = new EmptyDelegatingLoader("EmptyLoader3", null);
93+
EmptyDelegatingLoader emptyLoader4 = new EmptyDelegatingLoader("EmptyLoader4", emptyLoader3);
94+
ClassLoader named_child_3_cl = new TestClassLoader("ChildX", emptyLoader4); // Same names
95+
ClassLoader named_child_4_cl = new TestClassLoader("ChildX", emptyLoader4);
96+
ClassLoader named_child_5_cl = new TestClassLoader("ChildX", emptyLoader4);
97+
ClassLoader named_child_6_cl = new TestClassLoader("ChildX", emptyLoader4);
98+
loadTestClassInLoaderAndCheck("TestClass2", named_child_3_cl);
99+
loadTestClassInLoaderAndCheck("TestClass2", named_child_4_cl);
100+
loadTestClassInLoaderAndCheck("TestClass2", named_child_5_cl);
101+
loadTestClassInLoaderAndCheck("TestClass2", named_child_6_cl);
102+
103+
// D) Test output for several *unnamed* class loaders, same class, same parents,
104+
// and all these should be folded by default too.
105+
EmptyDelegatingLoader emptyLoader5 = new EmptyDelegatingLoader(null, null);
106+
EmptyDelegatingLoader emptyLoader6 = new EmptyDelegatingLoader(null, emptyLoader5);
107+
ClassLoader named_child_7_cl = new TestClassLoader(null, emptyLoader6); // Same names
108+
ClassLoader named_child_8_cl = new TestClassLoader(null, emptyLoader6);
109+
ClassLoader named_child_9_cl = new TestClassLoader(null, emptyLoader6);
110+
ClassLoader named_child_10_cl = new TestClassLoader(null, emptyLoader6);
111+
loadTestClassInLoaderAndCheck("TestClass2", named_child_7_cl);
112+
loadTestClassInLoaderAndCheck("TestClass2", named_child_8_cl);
113+
loadTestClassInLoaderAndCheck("TestClass2", named_child_9_cl);
114+
loadTestClassInLoaderAndCheck("TestClass2", named_child_10_cl);
80115

81116
// First test: simple output, no classes displayed
82117
OutputAnalyzer output = executor.execute("VM.classloaders");
83-
output.shouldContain("<bootstrap>");
84-
output.shouldMatch(".*TestClassLoader");
85-
output.shouldMatch("Kevin.*TestClassLoader");
86-
output.shouldMatch("Bill.*TestClassLoader");
118+
// (A)
119+
output.shouldContain("+-- <bootstrap>");
120+
output.shouldContain(" +-- \"platform\", jdk.internal.loader.ClassLoaders$PlatformClassLoader");
121+
output.shouldContain(" | +-- \"app\", jdk.internal.loader.ClassLoaders$AppClassLoader");
122+
output.shouldContain(" +-- \"Kevin\", ClassLoaderHierarchyTest$TestClassLoader");
123+
output.shouldContain(" +-- ClassLoaderHierarchyTest$TestClassLoader");
124+
output.shouldContain(" | +-- \"Bill\", ClassLoaderHierarchyTest$TestClassLoader");
125+
// (B)
126+
output.shouldContain(" +-- \"EmptyLoader1\", ClassLoaderHierarchyTest$EmptyDelegatingLoader");
127+
output.shouldContain(" | +-- \"EmptyLoader2\", ClassLoaderHierarchyTest$EmptyDelegatingLoader");
128+
output.shouldContain(" | +-- \"Child2\", ClassLoaderHierarchyTest$TestClassLoader");
129+
// (C)
130+
output.shouldContain(" +-- \"EmptyLoader3\", ClassLoaderHierarchyTest$EmptyDelegatingLoader");
131+
output.shouldContain(" | +-- \"EmptyLoader4\", ClassLoaderHierarchyTest$EmptyDelegatingLoader");
132+
output.shouldContain(" | +-- \"ChildX\", ClassLoaderHierarchyTest$TestClassLoader (+ 3 more)");
133+
// (D)
134+
output.shouldContain(" +-- ClassLoaderHierarchyTest$EmptyDelegatingLoader");
135+
output.shouldContain(" +-- ClassLoaderHierarchyTest$EmptyDelegatingLoader");
136+
output.shouldContain(" +-- ClassLoaderHierarchyTest$TestClassLoader (+ 3 more)");
87137

88138
// Second test: print with classes.
89139
output = executor.execute("VM.classloaders show-classes");
90140
output.shouldContain("<bootstrap>");
91141
output.shouldContain("java.lang.Object");
92-
output.shouldMatch(".*TestClassLoader");
93-
output.shouldMatch("Kevin.*TestClassLoader");
94-
output.shouldMatch("Bill.*TestClassLoader");
142+
output.shouldContain("java.lang.Enum");
143+
output.shouldContain("java.lang.NullPointerException");
95144
output.shouldContain("TestClass2");
145+
96146
output.shouldContain("Hidden Classes:");
97147

98148
Reference.reachabilityFence(unnamed_cl);

0 commit comments

Comments
 (0)