Mercurial > hg > release > heapstats-2.1

changeset 245:354b87b559ef

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Bug 3424: HeapStats Agent might crash when thread recorder is disabled via realoading configuration Reviewed-by: ykubota https://github.com/HeapStats/heapstats/pull/112
author Yasumasa Suenaga <yasuenag@gmail.com>
date Sat, 22 Jul 2017 19:04:47 +0900
parents 1e7c0fb4048a
children b3d1289e4539
files ChangeLog agent/src/heapstats-engines/callbackRegister.hpp agent/src/heapstats-engines/jniCallbackRegister.hpp agent/src/heapstats-engines/threadRecorder.cpp
diffstat 4 files changed, 157 insertions(+), 37 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Wed Jul 19 17:53:11 2017 +0900
+++ b/ChangeLog	Sat Jul 22 19:04:47 2017 +0900
@@ -1,3 +1,7 @@
+2017-07-22 Yasumasa Suenaga <yasuenag@gmail.com>
+
+	* Bug 3424: HeapStats Agent might crash when thread recorder is disabled via realoading configuration
+
 2017-07-19 Yasumasa Suenaga <yasuenag@gmail.com>
 
 	* Bug 3422: [REFACTORING] Stack and base pointer operation
--- a/agent/src/heapstats-engines/callbackRegister.hpp	Wed Jul 19 17:53:11 2017 +0900
+++ b/agent/src/heapstats-engines/callbackRegister.hpp	Sat Jul 22 19:04:47 2017 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file callbackRegister.hpp
  * \brief Handling JVMTI callback functions.
- * Copyright (C) 2015 Yasumasa Suenaga
+ * Copyright (C) 2015-2017 Yasumasa Suenaga
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -25,6 +25,8 @@
 #include <jvmti.h>
 #include <jni.h>
 
+#include <pthread.h>
+
 #include <list>
 
 /*!
@@ -38,6 +40,11 @@
    */
   static std::list<T> callbackList;
 
+  /*!
+   * \brief Read-Write lock for callback container.
+   */
+  static pthread_rwlock_t callbackLock;
+
  public:
   /*!
    * \brief Register JVMTI callback function.
@@ -45,7 +52,29 @@
    * \param callback [in] Callback function.
    */
   static void registerCallback(T callback) {
-    callbackList.push_back(callback);
+    pthread_rwlock_wrlock(&callbackLock);
+    {
+      callbackList.push_back(callback);
+    }
+    pthread_rwlock_unlock(&callbackLock);
+  };
+
+  /*!
+   * \brief Unregister JVMTI callback function.
+   * \param callback [in] Callback function to unregister.
+   */
+  static void unregisterCallback(T callback) {
+    pthread_rwlock_wrlock(&callbackLock);
+    {
+      for (auto itr = callbackList.cbegin();
+           itr != callbackList.cend(); itr++) {
+        if (*itr == callback) {
+          callbackList.erase(itr);
+          break;
+        }
+      }
+    }
+    pthread_rwlock_unlock(&callbackLock);
   };
 
   /*!
@@ -83,21 +112,36 @@
 template <typename T, jvmtiEvent event>
 std::list<T> TJVMTIEventCallback<T, event>::callbackList;
 
-#define ITERATE_CALLBACK_CHAIN(type, ...)                    \
-  for (std::list<type>::iterator itr = callbackList.begin(); \
-       itr != callbackList.end(); itr++) {                   \
-    (*itr)(__VA_ARGS__);                                     \
-  }
+/*!
+ * \brief Read-Write lock for callback container.
+ */
+template <typename T, jvmtiEvent event>
+pthread_rwlock_t TJVMTIEventCallback<T, event>::callbackLock =
+                                                     PTHREAD_RWLOCK_INITIALIZER;
+
+#define ITERATE_CALLBACK_CHAIN(type, ...)                      \
+  pthread_rwlock_rdlock(&callbackLock);                        \
+  {                                                            \
+    for (std::list<type>::iterator itr = callbackList.begin(); \
+         itr != callbackList.end(); itr++) {                   \
+      (*itr)(__VA_ARGS__);                                     \
+    }                                                          \
+  }                                                            \
+  pthread_rwlock_unlock(&callbackLock);
 
 #define DEFINE_MERGE_CALLBACK(callbackName)                   \
   static void mergeCallback(jvmtiEventCallbacks *callbacks) { \
-    if (callbackList.size() == 0) {                           \
-      callbacks->callbackName = NULL;                         \
-    } else if (callbackList.size() == 1) {                    \
-      callbacks->callbackName = *callbackList.begin();        \
-    } else {                                                  \
-      callbacks->callbackName = &callbackStub;                \
+    pthread_rwlock_rdlock(&callbackLock);                     \
+    {                                                         \
+      if (callbackList.size() == 0) {                         \
+        callbacks->callbackName = NULL;                       \
+      } else if (callbackList.size() == 1) {                  \
+        callbacks->callbackName = *callbackList.begin();      \
+      } else {                                                \
+        callbacks->callbackName = &callbackStub;              \
+      }                                                       \
     }                                                         \
+    pthread_rwlock_unlock(&callbackLock);                     \
   };
 
 /*!
--- a/agent/src/heapstats-engines/jniCallbackRegister.hpp	Wed Jul 19 17:53:11 2017 +0900
+++ b/agent/src/heapstats-engines/jniCallbackRegister.hpp	Sat Jul 22 19:04:47 2017 +0900
@@ -1,7 +1,7 @@
 /*!
  * \file jniCallbackRegister.hpp
  * \brief Handling JNI function callback.
- * Copyright (C) 2015 Yasumasa Suenaga
+ * Copyright (C) 2015-2017 Yasumasa Suenaga
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -25,6 +25,8 @@
 #include <jvmti.h>
 #include <jni.h>
 
+#include <pthread.h>
+
 #include <list>
 
 /* Type definition for JNI functions. */
@@ -51,6 +53,11 @@
    */
   static std::list<T> epilogueCallbackList;
 
+  /*!
+   * \brief Read-Write lock for callback container.
+   */
+  static pthread_rwlock_t callbackLock;
+
  public:
   /*!
    * \brief Register callbacks for JNI function.
@@ -59,13 +66,49 @@
    * \param epilogue [in] Callback after calling JNI function.
    */
   static void registerCallback(T prologue, T epilogue) {
-    if (prologue != NULL) {
-      prologueCallbackList.push_back(prologue);
+    pthread_rwlock_wrlock(&callbackLock);
+    {
+      if (prologue != NULL) {
+        prologueCallbackList.push_back(prologue);
+      }
+
+      if (epilogue != NULL) {
+        epilogueCallbackList.push_back(epilogue);
+      }
     }
+    pthread_rwlock_unlock(&callbackLock);
+  };
 
-    if (epilogue != NULL) {
-      epilogueCallbackList.push_back(epilogue);
+  /*!
+   * \brief Unegister callbacks for JNI function.
+   *
+   * \param prologue [in] Callback before calling JNI function.
+   * \param epilogue [in] Callback after calling JNI function.
+   */
+  static void unregisterCallback(T prologue, T epilogue) {
+    pthread_rwlock_wrlock(&callbackLock);
+    {
+      if (prologue != NULL) {
+        for (auto itr = prologueCallbackList.cbegin();
+             itr != prologueCallbackList.cend(); itr++) {
+          if (prologue == *itr) {
+            prologueCallbackList.erase(itr);
+            break;
+          }
+        }
+      }
+
+      if (epilogue != NULL) {
+        for (auto itr = epilogueCallbackList.cbegin();
+             itr != epilogueCallbackList.cend(); itr++) {
+          if (epilogue == *itr) {
+            epilogueCallbackList.erase(itr);
+            break;
+          }
+        }
+      }
     }
+    pthread_rwlock_unlock(&callbackLock);
   };
 };
 
@@ -81,20 +124,35 @@
 template <typename T>
 std::list<T> TJNICallbackRegister<T>::epilogueCallbackList;
 
+/*!
+ * \brief Read-Write lock for callback container.
+ */
+template <typename T>
+pthread_rwlock_t TJNICallbackRegister<T>::callbackLock =
+                                              PTHREAD_RWLOCK_INITIALIZER;
+
 #define ITERATE_JNI_CALLBACK_CHAIN(type, originalFunc, ...)                   \
   std::list<type>::iterator itr;                                              \
                                                                               \
-  for (itr = prologueCallbackList.begin(); itr != prologueCallbackList.end(); \
-       itr++) {                                                               \
-    (*itr)(__VA_ARGS__);                                                      \
+  pthread_rwlock_rdlock(&callbackLock);                                       \
+  {                                                                           \
+    for (itr = prologueCallbackList.begin();                                  \
+         itr != prologueCallbackList.end(); itr++) {                          \
+      (*itr)(__VA_ARGS__);                                                    \
+    }                                                                         \
   }                                                                           \
+  pthread_rwlock_unlock(&callbackLock);                                       \
                                                                               \
   originalFunc(__VA_ARGS__);                                                  \
                                                                               \
-  for (itr = epilogueCallbackList.begin(); itr != epilogueCallbackList.end(); \
-       itr++) {                                                               \
-    (*itr)(__VA_ARGS__);                                                      \
-  }
+  pthread_rwlock_rdlock(&callbackLock);                                       \
+  {                                                                           \
+    for (itr = epilogueCallbackList.begin();                                  \
+         itr != epilogueCallbackList.end(); itr++) {                          \
+      (*itr)(__VA_ARGS__);                                                    \
+    }                                                                         \
+  }                                                                           \
+  pthread_rwlock_unlock(&callbackLock);
 
 /*!
  * \brief JVM_Sleep callback (java.lang.System#sleep())
--- a/agent/src/heapstats-engines/threadRecorder.cpp	Wed Jul 19 17:53:11 2017 +0900
+++ b/agent/src/heapstats-engines/threadRecorder.cpp	Sat Jul 22 19:04:47 2017 +0900
@@ -614,17 +614,12 @@
  * \param buf_sz [in] Size of ring buffer.
  */
 void TThreadRecorder::initialize(jvmtiEnv *jvmti, JNIEnv *env, size_t buf_sz) {
-  static bool isRegistered = false;
-
   if (likely(inst == NULL)) {
     inst = new TThreadRecorder(buf_sz);
 
-    if (!isRegistered) {
-      isRegistered = true;
-      registerHookPoint(jvmti, env);
-      registerJNIHookPoint(env);
-      registerIOTracer(jvmti, env);
-    }
+    registerHookPoint(jvmti, env);
+    registerJNIHookPoint(env);
+    registerIOTracer(jvmti, env);
 
     inst->registerAllThreads(jvmti);
   }
@@ -694,20 +689,39 @@
  */
 void TThreadRecorder::finalize(jvmtiEnv *jvmti, JNIEnv *env,
                                const char *fname) {
-  /* Stop to hook JVMTI event */
+  /* Stop JVMTI events which are used by ThreadRecorder only. */
   TThreadStartCallback::switchEventNotification(jvmti, JVMTI_DISABLE);
   TThreadEndCallback::switchEventNotification(jvmti, JVMTI_DISABLE);
-  TMonitorContendedEnterCallback::switchEventNotification(jvmti, JVMTI_DISABLE);
   TMonitorContendedEnteredCallback::switchEventNotification(jvmti,
                                                             JVMTI_DISABLE);
   TMonitorWaitCallback::switchEventNotification(jvmti, JVMTI_DISABLE);
   TMonitorWaitedCallback::switchEventNotification(jvmti, JVMTI_DISABLE);
 
-  /* Stop to hook JNI function */
+  /* Stop JNI function hooks. */
   TJVMSleepCallback::switchCallback(env, false);
   TUnsafeParkCallback::switchCallback(env, false);
 
-  /* Stop to hook IoTrace */
+  /* Unregister JVMTI event callbacks which are used by ThreadRecorder. */
+  TThreadStartCallback::unregisterCallback(&OnThreadStart);
+  TThreadEndCallback::unregisterCallback(&OnThreadEnd);
+  TMonitorContendedEnterCallback::unregisterCallback(
+      &OnMonitorContendedEnterForThreadRecording);
+  TMonitorContendedEnteredCallback::unregisterCallback(
+      &OnMonitorContendedEnteredForThreadRecording);
+  TMonitorWaitCallback::unregisterCallback(&OnMonitorWait);
+  TMonitorWaitedCallback::unregisterCallback(&OnMonitorWaited);
+  TDataDumpRequestCallback::unregisterCallback(
+      &OnDataDumpRequestForDumpThreadRecordData);
+
+  /* Refresh JVMTI event callbacks */
+  registerJVMTICallbacks(jvmti);
+
+  /* Unregister JNI function hooks which are used by ThreadRecorder. */
+  TJVMSleepCallback::unregisterCallback(&JVM_SleepPrologue, &JVM_SleepEpilogue);
+  TUnsafeParkCallback::unregisterCallback(&UnsafeParkPrologue,
+                                          &UnsafeParkEpilogue);
+
+  /* Stop IoTrace hook */
   UnregisterIOTracer(env);
 
   /* Wait until all tasks are finished. */