Skip to content

Commit 1c33847

Browse files
committed
8259067: bootclasspath append takes out object lock
Reviewed-by: lfoltan, sspitsyn, iklam, dholmes
1 parent 0e6de4e commit 1c33847

File tree

6 files changed

+32
-26
lines changed

6 files changed

+32
-26
lines changed

src/hotspot/share/classfile/classLoader.cpp

+13-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -141,8 +141,9 @@ PerfCounter* ClassLoader::_unsafe_defineClassCallCounter = NULL;
141141
GrowableArray<ModuleClassPathList*>* ClassLoader::_patch_mod_entries = NULL;
142142
GrowableArray<ModuleClassPathList*>* ClassLoader::_exploded_entries = NULL;
143143
ClassPathEntry* ClassLoader::_jrt_entry = NULL;
144-
ClassPathEntry* ClassLoader::_first_append_entry = NULL;
145-
ClassPathEntry* ClassLoader::_last_append_entry = NULL;
144+
145+
ClassPathEntry* volatile ClassLoader::_first_append_entry_list = NULL;
146+
ClassPathEntry* volatile ClassLoader::_last_append_entry = NULL;
146147
#if INCLUDE_CDS
147148
ClassPathEntry* ClassLoader::_app_classpath_entries = NULL;
148149
ClassPathEntry* ClassLoader::_last_app_classpath_entry = NULL;
@@ -815,7 +816,7 @@ ClassPathZipEntry* ClassLoader::create_class_path_zip_entry(const char *path, bo
815816

816817
// returns true if entry already on class path
817818
bool ClassLoader::contains_append_entry(const char* name) {
818-
ClassPathEntry* e = _first_append_entry;
819+
ClassPathEntry* e = first_append_entry();
819820
while (e != NULL) {
820821
// assume zip entries have been canonicalized
821822
if (strcmp(name, e->name()) == 0) {
@@ -826,11 +827,14 @@ bool ClassLoader::contains_append_entry(const char* name) {
826827
return false;
827828
}
828829

830+
// The boot append entries are added with a lock, and read lock free.
829831
void ClassLoader::add_to_boot_append_entries(ClassPathEntry *new_entry) {
830832
if (new_entry != NULL) {
833+
MutexLocker ml(Bootclasspath_lock, Mutex::_no_safepoint_check_flag);
831834
if (_last_append_entry == NULL) {
832-
assert(_first_append_entry == NULL, "boot loader's append class path entry list not empty");
833-
_first_append_entry = _last_append_entry = new_entry;
835+
_last_append_entry = new_entry;
836+
assert(first_append_entry() == NULL, "boot loader's append class path entry list not empty");
837+
Atomic::release_store(&_first_append_entry_list, new_entry);
834838
} else {
835839
_last_append_entry->set_next(new_entry);
836840
_last_append_entry = new_entry;
@@ -944,7 +948,7 @@ void ClassLoader::print_bootclasspath() {
944948
}
945949

946950
// appended entries
947-
e = _first_append_entry;
951+
e = first_append_entry();
948952
while (e != NULL) {
949953
tty->print("%s ;", e->name());
950954
e = e->next();
@@ -1252,7 +1256,7 @@ InstanceKlass* ClassLoader::load_class(Symbol* name, bool search_append_only, TR
12521256
assert(classpath_index == 0, "The classpath_index has been incremented incorrectly");
12531257
classpath_index = 1;
12541258

1255-
e = _first_append_entry;
1259+
e = first_append_entry();
12561260
while (e != NULL) {
12571261
stream = e->open_stream(file_name, CHECK_NULL);
12581262
if (NULL != stream) {
@@ -1427,7 +1431,7 @@ void ClassLoader::record_result(InstanceKlass* ik, const ClassFileStream* stream
14271431
// Initialize the class loader's access to methods in libzip. Parse and
14281432
// process the boot classpath into a list ClassPathEntry objects. Once
14291433
// this list has been created, it must not change order (see class PackageInfo)
1430-
// it can be appended to and is by jvmti and the kernel vm.
1434+
// it can be appended to and is by jvmti.
14311435

14321436
void ClassLoader::initialize() {
14331437
EXCEPTION_MARK;

src/hotspot/share/classfile/classLoader.hpp

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -53,6 +53,7 @@ class ClassPathEntry : public CHeapObj<mtClass> {
5353
ClassPathEntry* next() const;
5454
virtual ~ClassPathEntry() {}
5555
void set_next(ClassPathEntry* next);
56+
5657
virtual bool is_modules_image() const { return false; }
5758
virtual bool is_jar_file() const { return false; }
5859
// Is this entry created from the "Class-path" attribute from a JAR Manifest?
@@ -214,9 +215,13 @@ class ClassLoader: AllStatic {
214215
// 3. the boot loader's append path
215216
// [-Xbootclasspath/a]; [jvmti appended entries]
216217
// Note: boot loader append path does not support named modules.
217-
static ClassPathEntry* _first_append_entry;
218+
static ClassPathEntry* volatile _first_append_entry_list;
219+
static ClassPathEntry* first_append_entry() {
220+
return Atomic::load_acquire(&_first_append_entry_list);
221+
}
222+
218223
// Last entry in linked list of appended ClassPathEntry instances
219-
static ClassPathEntry* _last_append_entry;
224+
static ClassPathEntry* volatile _last_append_entry;
220225

221226
// Info used by CDS
222227
CDS_ONLY(static ClassPathEntry* _app_classpath_entries;)
@@ -234,7 +239,7 @@ class ClassLoader: AllStatic {
234239
CDS_ONLY(static ClassPathEntry* app_classpath_entries() {return _app_classpath_entries;})
235240
CDS_ONLY(static ClassPathEntry* module_path_entries() {return _module_path_entries;})
236241

237-
static bool has_bootclasspath_append() { return _first_append_entry != NULL; }
242+
static bool has_bootclasspath_append() { return first_append_entry() != NULL; }
238243

239244
protected:
240245
// Initialization:

src/hotspot/share/classfile/classLoader.inline.hpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -48,7 +48,7 @@ inline ClassPathEntry* ClassLoader::classpath_entry(int n) {
4848
// the _jrt_entry is not included in the _first_append_entry
4949
// linked list, it must be accounted for when comparing the
5050
// class path vs. the shared archive class path.
51-
ClassPathEntry* e = ClassLoader::_first_append_entry;
51+
ClassPathEntry* e = first_append_entry();
5252
while (--n >= 1) {
5353
assert(e != NULL, "Not that many classpath entries.");
5454
e = e->next();
@@ -72,7 +72,7 @@ inline int ClassLoader::num_boot_classpath_entries() {
7272
Arguments::assert_is_dumping_archive();
7373
assert(has_jrt_entry(), "must have a java runtime image");
7474
int num_entries = 1; // count the runtime image
75-
ClassPathEntry* e = ClassLoader::_first_append_entry;
75+
ClassPathEntry* e = first_append_entry();
7676
while (e != NULL) {
7777
num_entries ++;
7878
e = e->next();
@@ -82,7 +82,7 @@ inline int ClassLoader::num_boot_classpath_entries() {
8282

8383
inline ClassPathEntry* ClassLoader::get_next_boot_classpath_entry(ClassPathEntry* e) {
8484
if (e == ClassLoader::_jrt_entry) {
85-
return ClassLoader::_first_append_entry;
85+
return first_append_entry();
8686
} else {
8787
return e->next();
8888
}

src/hotspot/share/prims/jvmtiEnv.cpp

+1-8
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -659,13 +659,6 @@ JvmtiEnv::AddToBootstrapClassLoaderSearch(const char* segment) {
659659
return JVMTI_ERROR_ILLEGAL_ARGUMENT;
660660
}
661661

662-
// lock the loader
663-
Thread* thread = Thread::current();
664-
HandleMark hm(thread);
665-
Handle loader_lock = Handle(thread, SystemDictionary::system_loader_lock());
666-
667-
ObjectLocker ol(loader_lock, thread);
668-
669662
// add the jar file to the bootclasspath
670663
log_info(class, load)("opened: %s", zip_entry->name());
671664
#if INCLUDE_CDS

src/hotspot/share/runtime/mutexLocker.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ Mutex* CDSLambda_lock = NULL;
157157
Mutex* DumpRegion_lock = NULL;
158158
Mutex* ClassListFile_lock = NULL;
159159
#endif // INCLUDE_CDS
160+
Mutex* Bootclasspath_lock = NULL;
160161

161162
#if INCLUDE_JVMCI
162163
Monitor* JVMCI_lock = NULL;
@@ -353,6 +354,7 @@ void mutex_init() {
353354
def(DumpRegion_lock , PaddedMutex , leaf, true, _safepoint_check_never);
354355
def(ClassListFile_lock , PaddedMutex , leaf, true, _safepoint_check_never);
355356
#endif // INCLUDE_CDS
357+
def(Bootclasspath_lock , PaddedMutex , leaf, false, _safepoint_check_never);
356358

357359
#if INCLUDE_JVMCI
358360
def(JVMCI_lock , PaddedMonitor, nonleaf+2, true, _safepoint_check_always);

src/hotspot/share/runtime/mutexLocker.hpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -158,6 +158,8 @@ extern Mutex* CodeHeapStateAnalytics_lock; // lock print functions against
158158
extern Monitor* JVMCI_lock; // Monitor to control initialization of JVMCI
159159
#endif
160160

161+
extern Mutex* Bootclasspath_lock;
162+
161163
extern Mutex* tty_lock; // lock to synchronize output.
162164

163165
// A MutexLocker provides mutual exclusion with respect to a given mutex

0 commit comments

Comments
 (0)