From 5bf1b9a53485b571095b934f65c2d6143f748d4f Mon Sep 17 00:00:00 2001
From: Aleksey Shipilev <shade@openjdk.org>
Date: Thu, 25 May 2023 19:02:09 +0000
Subject: [PATCH] 8283520: JFR: Memory leak in dcmd_arena

Backport-of: 6a8be358d2af34fab8798077202b998badaa5d54
---
 src/hotspot/share/jfr/dcmd/jfrDcmds.cpp        | 17 ++++++++---------
 .../share/jfr/support/jfrThreadLocal.cpp       | 18 ++++++++++++++++++
 .../share/jfr/support/jfrThreadLocal.hpp       |  4 ++++
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp b/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp
index 875e7f8e475..a21a4fa7d48 100644
--- a/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp
+++ b/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp
@@ -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"
@@ -311,12 +312,10 @@ 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) {
@@ -324,7 +323,7 @@ static char* dcmd_arena_allocate(size_t size) {
   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) {
@@ -345,7 +344,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) {
@@ -394,7 +393,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");
diff --git a/src/hotspot/share/jfr/support/jfrThreadLocal.cpp b/src/hotspot/share/jfr/support/jfrThreadLocal.cpp
index 48b6d657ebc..1c4fabc75c3 100644
--- a/src/hotspot/share/jfr/support/jfrThreadLocal.cpp
+++ b/src/hotspot/share/jfr/support/jfrThreadLocal.cpp
@@ -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"
@@ -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),
@@ -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) {
@@ -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;
+}
diff --git a/src/hotspot/share/jfr/support/jfrThreadLocal.hpp b/src/hotspot/share/jfr/support/jfrThreadLocal.hpp
index 7e4a0819cf2..ab7d6170cdb 100644
--- a/src/hotspot/share/jfr/support/jfrThreadLocal.hpp
+++ b/src/hotspot/share/jfr/support/jfrThreadLocal.hpp
@@ -28,6 +28,7 @@
 #include "jfr/utilities/jfrBlob.hpp"
 #include "jfr/utilities/jfrTypes.hpp"
 
+class Arena;
 class JavaThread;
 class JfrBuffer;
 class JfrStackFrame;
@@ -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;
@@ -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;
-- 
2.43.2