Skip to content

Commit

Permalink
8283520: JFR: Memory leak in dcmd_arena
Browse files Browse the repository at this point in the history
Backport-of: 6a8be358d2af34fab8798077202b998badaa5d54
  • Loading branch information
shipilev committed May 25, 2023
1 parent c581886 commit 5bf1b9a
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 9 deletions.
17 changes: 8 additions & 9 deletions src/hotspot/share/jfr/dcmd/jfrDcmds.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "jfr/jni/jfrJavaSupport.hpp"
#include "jfr/recorder/jfrRecorder.hpp"
#include "jfr/recorder/service/jfrOptionSet.hpp"
#include "jfr/support/jfrThreadLocal.hpp"
#include "logging/log.hpp"
#include "logging/logConfiguration.hpp"
#include "logging/logMessage.hpp"
Expand Down Expand Up @@ -294,20 +295,18 @@ static void initialize_dummy_descriptors(GrowableArray<DCmdArgumentInfo*>* array
// we keep them in a thread local arena. The arena is reset between invocations.
static THREAD_LOCAL Arena* dcmd_arena = NULL;

static void prepare_dcmd_string_arena() {
if (dcmd_arena == NULL) {
dcmd_arena = new (mtTracing) Arena(mtTracing);
} else {
dcmd_arena->destruct_contents(); // will grow on next allocation
}
static void prepare_dcmd_string_arena(JavaThread* jt) {
dcmd_arena = JfrThreadLocal::dcmd_arena(jt);
assert(dcmd_arena != nullptr, "invariant");
dcmd_arena->destruct_contents(); // will grow on next allocation
}

static char* dcmd_arena_allocate(size_t size) {
assert(dcmd_arena != NULL, "invariant");
return (char*)dcmd_arena->Amalloc(size);
}

static const char* get_as_dcmd_arena_string(oop string, JavaThread* t) {
static const char* get_as_dcmd_arena_string(oop string) {
char* str = NULL;
const typeArrayOop value = java_lang_String::value(string);
if (value != NULL) {
Expand All @@ -328,7 +327,7 @@ static const char* read_string_field(oop argument, const char* field_name, TRAPS
args.set_receiver(argument);
JfrJavaSupport::get_field(&args, THREAD);
const oop string_oop = result.get_oop();
return string_oop != NULL ? get_as_dcmd_arena_string(string_oop, (JavaThread*)THREAD) : NULL;
return string_oop != NULL ? get_as_dcmd_arena_string(string_oop) : NULL;
}

static bool read_boolean_field(oop argument, const char* field_name, TRAPS) {
Expand Down Expand Up @@ -377,7 +376,7 @@ GrowableArray<DCmdArgumentInfo*>* JfrDCmd::argument_info_array() const {
assert(arguments->is_array(), "must be array");
const int num_arguments = arguments->length();
assert(num_arguments == _num_arguments, "invariant");
prepare_dcmd_string_arena();
prepare_dcmd_string_arena(thread);
for (int i = 0; i < num_arguments; ++i) {
DCmdArgumentInfo* const dai = create_info(arguments->obj_at(i), thread);
assert(dai != NULL, "invariant");
Expand Down
18 changes: 18 additions & 0 deletions src/hotspot/share/jfr/support/jfrThreadLocal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "jfr/recorder/storage/jfrStorage.hpp"
#include "jfr/support/jfrThreadLocal.hpp"
#include "memory/allocation.inline.hpp"
#include "memory/arena.hpp"
#include "runtime/os.hpp"
#include "runtime/thread.inline.hpp"
#include "utilities/sizes.hpp"
Expand All @@ -46,6 +47,7 @@ JfrThreadLocal::JfrThreadLocal() :
_load_barrier_buffer_epoch_0(NULL),
_load_barrier_buffer_epoch_1(NULL),
_stackframes(NULL),
_dcmd_arena(nullptr),
_trace_id(JfrTraceId::assign_thread_id()),
_thread(),
_data_lost(0),
Expand Down Expand Up @@ -142,6 +144,10 @@ void JfrThreadLocal::release(Thread* t) {
_load_barrier_buffer_epoch_1->set_retired();
_load_barrier_buffer_epoch_1 = NULL;
}
if (_dcmd_arena != nullptr) {
delete _dcmd_arena;
_dcmd_arena = nullptr;
}
}

void JfrThreadLocal::release(JfrThreadLocal* tl, Thread* t) {
Expand Down Expand Up @@ -218,3 +224,15 @@ void JfrThreadLocal::include(Thread* t) {
u4 JfrThreadLocal::stackdepth() const {
return _stackdepth != 0 ? _stackdepth : (u4)JfrOptionSet::stackdepth();
}

Arena* JfrThreadLocal::dcmd_arena(JavaThread* jt) {
assert(jt != nullptr, "invariant");
JfrThreadLocal* tl = jt->jfr_thread_local();
Arena* arena = tl->_dcmd_arena;
if (arena != nullptr) {
return arena;
}
arena = new (mtTracing) Arena(mtTracing);
tl->_dcmd_arena = arena;
return arena;
}
4 changes: 4 additions & 0 deletions src/hotspot/share/jfr/support/jfrThreadLocal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "jfr/utilities/jfrBlob.hpp"
#include "jfr/utilities/jfrTypes.hpp"

class Arena;
class JavaThread;
class JfrBuffer;
class JfrStackFrame;
Expand All @@ -42,6 +43,7 @@ class JfrThreadLocal {
JfrBuffer* _load_barrier_buffer_epoch_0;
JfrBuffer* _load_barrier_buffer_epoch_1;
mutable JfrStackFrame* _stackframes;
Arena* _dcmd_arena;
mutable traceid _trace_id;
JfrBlobHandle _thread;
u8 _data_lost;
Expand Down Expand Up @@ -219,6 +221,8 @@ class JfrThreadLocal {
return _dead;
}

static Arena* dcmd_arena(JavaThread* jt);

bool has_thread_blob() const;
void set_thread_blob(const JfrBlobHandle& handle);
const JfrBlobHandle& thread_blob() const;
Expand Down

1 comment on commit 5bf1b9a

@openjdk-notifier
Copy link

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.