Skip to content

Commit edcc559

Browse files
author
Thomas Schatzl
committed
8316661: CompilerThread leaks CodeBlob memory when dynamically stopping compiler thread in non-product
Reviewed-by: kvn, thartmann
1 parent 1be3557 commit edcc559

File tree

6 files changed

+26
-39
lines changed

6 files changed

+26
-39
lines changed

src/hotspot/share/code/codeBlob.hpp

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,6 @@ class CodeBlob {
119119
#ifndef PRODUCT
120120
AsmRemarks _asm_remarks;
121121
DbgStrings _dbg_strings;
122-
123-
~CodeBlob() {
124-
_asm_remarks.clear();
125-
_dbg_strings.clear();
126-
}
127122
#endif // not PRODUCT
128123

129124
CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& layout, int frame_complete_offset,
@@ -132,10 +127,17 @@ class CodeBlob {
132127
CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& layout, CodeBuffer* cb, int frame_complete_offset,
133128
int frame_size, OopMapSet* oop_maps,
134129
bool caller_must_gc_arguments, bool compiled = false);
130+
131+
void operator delete(void* p) { }
132+
135133
public:
136134
// Only used by unit test.
137135
CodeBlob() : _type(compiler_none) {}
138136

137+
virtual ~CodeBlob() {
138+
assert(_oop_maps == nullptr, "Not flushed");
139+
}
140+
139141
// Returns the space needed for CodeBlob
140142
static unsigned int allocation_size(CodeBuffer* cb, int header_size);
141143
static unsigned int align_code_offset(int offset);
@@ -404,10 +406,6 @@ class BufferBlob: public RuntimeBlob {
404406
BufferBlob(const char* name, int size);
405407
BufferBlob(const char* name, int size, CodeBuffer* cb);
406408

407-
// This ordinary operator delete is needed even though not used, so the
408-
// below two-argument operator delete will be treated as a placement
409-
// delete rather than an ordinary sized delete; see C++14 3.7.4.2/p2.
410-
void operator delete(void* p);
411409
void* operator new(size_t s, unsigned size) throw();
412410

413411
public:
@@ -492,10 +490,6 @@ class RuntimeStub: public RuntimeBlob {
492490
bool caller_must_gc_arguments
493491
);
494492

495-
// This ordinary operator delete is needed even though not used, so the
496-
// below two-argument operator delete will be treated as a placement
497-
// delete rather than an ordinary sized delete; see C++14 3.7.4.2/p2.
498-
void operator delete(void* p);
499493
void* operator new(size_t s, unsigned size) throw();
500494

501495
public:
@@ -532,10 +526,6 @@ class SingletonBlob: public RuntimeBlob {
532526
friend class VMStructs;
533527

534528
protected:
535-
// This ordinary operator delete is needed even though not used, so the
536-
// below two-argument operator delete will be treated as a placement
537-
// delete rather than an ordinary sized delete; see C++14 3.7.4.2/p2.
538-
void operator delete(void* p);
539529
void* operator new(size_t s, unsigned size) throw();
540530

541531
public:
@@ -750,10 +740,6 @@ class UpcallStub: public RuntimeBlob {
750740
intptr_t exception_handler_offset,
751741
jobject receiver, ByteSize frame_data_offset);
752742

753-
// This ordinary operator delete is needed even though not used, so the
754-
// below two-argument operator delete will be treated as a placement
755-
// delete rather than an ordinary sized delete; see C++14 3.7.4.2/p2.
756-
void operator delete(void* p);
757743
void* operator new(size_t s, unsigned size) throw();
758744

759745
struct FrameData {

src/hotspot/share/code/codeCache.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,10 +472,10 @@ CodeHeap* CodeCache::get_code_heap_containing(void* start) {
472472
return nullptr;
473473
}
474474

475-
CodeHeap* CodeCache::get_code_heap(const CodeBlob* cb) {
475+
CodeHeap* CodeCache::get_code_heap(const void* cb) {
476476
assert(cb != nullptr, "CodeBlob is null");
477477
FOR_ALL_HEAPS(heap) {
478-
if ((*heap)->contains_blob(cb)) {
478+
if ((*heap)->contains(cb)) {
479479
return *heap;
480480
}
481481
}
@@ -604,6 +604,7 @@ void CodeCache::free(CodeBlob* cb) {
604604
heap->set_adapter_count(heap->adapter_count() - 1);
605605
}
606606

607+
cb->~CodeBlob();
607608
// Get heap for given CodeBlob and deallocate
608609
get_code_heap(cb)->deallocate(cb);
609610

src/hotspot/share/code/codeCache.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ class CodeCache : AllStatic {
117117
// Creates a new heap with the given name and size, containing CodeBlobs of the given type
118118
static void add_heap(ReservedSpace rs, const char* name, CodeBlobType code_blob_type);
119119
static CodeHeap* get_code_heap_containing(void* p); // Returns the CodeHeap containing the given pointer, or nullptr
120-
static CodeHeap* get_code_heap(const CodeBlob* cb); // Returns the CodeHeap for the given CodeBlob
120+
static CodeHeap* get_code_heap(const void* cb); // Returns the CodeHeap for the given CodeBlob
121121
static CodeHeap* get_code_heap(CodeBlobType code_blob_type); // Returns the CodeHeap for the given CodeBlobType
122122
// Returns the name of the VM option to set the size of the corresponding CodeHeap
123123
static const char* get_code_heap_flag_name(CodeBlobType code_blob_type);
@@ -397,10 +397,10 @@ template <class T, class Filter, bool is_relaxed> class CodeBlobIterator : publi
397397
// If set to nullptr, initialized by first call to next()
398398
_code_blob = nm;
399399
if (nm != nullptr) {
400-
while(!(*_heap)->contains_blob(_code_blob)) {
400+
while(!(*_heap)->contains(_code_blob)) {
401401
++_heap;
402402
}
403-
assert((*_heap)->contains_blob(_code_blob), "match not found");
403+
assert((*_heap)->contains(_code_blob), "match not found");
404404
}
405405
}
406406

src/hotspot/share/compiler/compileBroker.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1770,17 +1770,22 @@ bool CompileBroker::init_compiler_runtime() {
17701770
return true;
17711771
}
17721772

1773+
void CompileBroker::free_buffer_blob_if_allocated(CompilerThread* thread) {
1774+
BufferBlob* blob = thread->get_buffer_blob();
1775+
if (blob != nullptr) {
1776+
blob->flush();
1777+
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
1778+
CodeCache::free(blob);
1779+
}
1780+
}
1781+
17731782
/**
17741783
* If C1 and/or C2 initialization failed, we shut down all compilation.
17751784
* We do this to keep things simple. This can be changed if it ever turns
17761785
* out to be a problem.
17771786
*/
17781787
void CompileBroker::shutdown_compiler_runtime(AbstractCompiler* comp, CompilerThread* thread) {
1779-
// Free buffer blob, if allocated
1780-
if (thread->get_buffer_blob() != nullptr) {
1781-
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
1782-
CodeCache::free(thread->get_buffer_blob());
1783-
}
1788+
free_buffer_blob_if_allocated(thread);
17841789

17851790
if (comp->should_perform_shutdown()) {
17861791
// There are two reasons for shutting down the compiler
@@ -1919,11 +1924,7 @@ void CompileBroker::compiler_thread_loop() {
19191924
// Notify compiler that the compiler thread is about to stop
19201925
thread->compiler()->stopping_compiler_thread(thread);
19211926

1922-
// Free buffer blob, if allocated
1923-
if (thread->get_buffer_blob() != nullptr) {
1924-
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
1925-
CodeCache::free(thread->get_buffer_blob());
1926-
}
1927+
free_buffer_blob_if_allocated(thread);
19271928
return; // Stop this thread.
19281929
}
19291930
}

src/hotspot/share/compiler/compileBroker.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,8 @@ class CompileBroker: AllStatic {
252252
static bool wait_for_jvmci_completion(JVMCICompiler* comp, CompileTask* task, JavaThread* thread);
253253
#endif
254254

255+
static void free_buffer_blob_if_allocated(CompilerThread* thread);
256+
255257
static void invoke_compiler_on_method(CompileTask* task);
256258
static void handle_compile_error(CompilerThread* thread, CompileTask* task, ciEnv* ci_env,
257259
int compilable, const char* failure_reason);

src/hotspot/share/memory/heap.hpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,6 @@ class CodeHeap : public CHeapObj<mtCode> {
171171

172172
// Containment means "contained in committed space".
173173
bool contains(const void* p) const { return low() <= p && p < high(); }
174-
bool contains_blob(const CodeBlob* blob) const {
175-
return contains((void*)blob);
176-
}
177174

178175
void* find_start(void* p) const; // returns the block containing p or null
179176
CodeBlob* find_blob(void* start) const;

0 commit comments

Comments
 (0)