Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8253262: Allocate in DumpRegion is not thread safe
Reviewed-by: ccheung
  • Loading branch information
yminqi committed Sep 17, 2020
1 parent 3570f5a commit 12dfe1c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
16 changes: 9 additions & 7 deletions src/hotspot/share/oops/symbol.cpp
Expand Up @@ -35,6 +35,7 @@
#include "memory/universe.hpp"
#include "oops/symbol.hpp"
#include "runtime/atomic.hpp"
#include "runtime/mutexLocker.hpp"
#include "runtime/os.hpp"
#include "runtime/signature.hpp"
#include "utilities/utf8.hpp"
Expand Down Expand Up @@ -63,13 +64,14 @@ Symbol::Symbol(const u1* name, int length, int refcount) {
void* Symbol::operator new(size_t sz, int len) throw() {
#if INCLUDE_CDS
if (DumpSharedSpaces) {
// To get deterministic output from -Xshare:dump, we ensure that Symbols are allocated in
// increasing addresses. When the symbols are copied into the archive, we preserve their
// relative address order (see SortedSymbolClosure in metaspaceShared.cpp)
//
// We cannot use arena because arena chunks are allocated by the OS. As a result, for example,
// the archived symbol of "java/lang/Object" may sometimes be lower than "java/lang/String", and
// sometimes be higher. This would cause non-deterministic contents in the archive.
MutexLocker ml(DumpRegion_lock, Mutex::_no_safepoint_check_flag);
// To get deterministic output from -Xshare:dump, we ensure that Symbols are allocated in
// increasing addresses. When the symbols are copied into the archive, we preserve their
// relative address order (sorted, see ArchiveBuilder::gather_klasses_and_symbols).
//
// We cannot use arena because arena chunks are allocated by the OS. As a result, for example,
// the archived symbol of "java/lang/Object" may sometimes be lower than "java/lang/String", and
// sometimes be higher. This would cause non-deterministic contents in the archive.
DEBUG_ONLY(static void* last = 0);
void* p = (void*)MetaspaceShared::symbol_space_alloc(size(len)*wordSize);
assert(p > last, "must increase monotonically");
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/runtime/mutexLocker.cpp
Expand Up @@ -151,6 +151,7 @@ Mutex* CDSClassFileStream_lock = NULL;
#endif
Mutex* DumpTimeTable_lock = NULL;
Mutex* CDSLambda_lock = NULL;
Mutex* DumpRegion_lock = NULL;
#endif // INCLUDE_CDS

#if INCLUDE_JVMCI
Expand Down Expand Up @@ -346,6 +347,7 @@ void mutex_init() {
#endif
def(DumpTimeTable_lock , PaddedMutex , leaf - 1, true, _safepoint_check_never);
def(CDSLambda_lock , PaddedMutex , leaf, true, _safepoint_check_never);
def(DumpRegion_lock , PaddedMutex , leaf, true, _safepoint_check_never);
#endif // INCLUDE_CDS

#if INCLUDE_JVMCI
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/runtime/mutexLocker.hpp
Expand Up @@ -130,6 +130,7 @@ extern Mutex* CDSClassFileStream_lock; // FileMapInfo::open_stream_for
#endif
extern Mutex* DumpTimeTable_lock; // SystemDictionaryShared::find_or_allocate_info_for
extern Mutex* CDSLambda_lock; // SystemDictionaryShared::get_shared_lambda_proxy_class
extern Mutex* DumpRegion_lock; // Symbol::operator new(size_t sz, int len)
#endif // INCLUDE_CDS
#if INCLUDE_JFR
extern Mutex* JfrStacktrace_lock; // used to guard access to the JFR stacktrace table
Expand Down

1 comment on commit 12dfe1c

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on 12dfe1c Sep 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.