Mercurial > hg > heapstats

changeset 151:b59588adef89

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Bug 2870: JVM crash when oneway option is changed through Reviewed-by: ykubota GitHub: https://github.com/HeapStats/heapstats/pull/19
author Yasumasa Suenaga <yasuenag@gmail.com>
date Sat, 05 Mar 2016 22:45:53 +0900
parents 163841d800b9
children ce583e804b7a
files ChangeLog agent/src/heapstats-engines/configuration.cpp agent/src/heapstats-engines/configuration.hpp agent/src/heapstats-engines/heapstatsMBean.cpp
diffstat 4 files changed, 106 insertions(+), 73 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Fri Feb 26 21:58:02 2016 +0900
+++ b/ChangeLog	Sat Mar 05 22:45:53 2016 +0900
@@ -1,3 +1,7 @@
+2016-03-05  Yasumasa Suenaga <yasuenag@gmail.com>
+
+	* Bug 2870: JVM crash when oneway option is changed through JMX.
+
 2016-02-26  Yasumasa Suenaga <yasuenag@gmail.com>
 
 	* Bug 2861: Avoid hiding overloaded virtual function warning.
--- a/agent/src/heapstats-engines/configuration.cpp	Fri Feb 26 21:58:02 2016 +0900
+++ b/agent/src/heapstats-engines/configuration.cpp	Sat Mar 05 22:45:53 2016 +0900
@@ -54,12 +54,14 @@
  */
 TConfiguration::TConfiguration(const TConfiguration &src) {
   jvmInfo = src.jvmInfo;
-  isLoaded = src.isLoaded;
+  isLoaded = false; // Set to false to load configuration from src.
   alertThreshold = src.alertThreshold;
   heapAlertThreshold = src.heapAlertThreshold;
 
   /* Initialize each configurations. */
   initializeConfig(&src);
+
+  isLoaded = src.isLoaded;
 }
 
 /*!
--- a/agent/src/heapstats-engines/configuration.hpp	Fri Feb 26 21:58:02 2016 +0900
+++ b/agent/src/heapstats-engines/configuration.hpp	Sat Mar 05 22:45:53 2016 +0900
@@ -72,8 +72,8 @@
 template <typename T, TConfigDataType V>
 class TConfigElement : public TConfigElementSuper {
  public:
-  typedef void (*TSetter)(TConfiguration *inst, T value, T *dest);
-  typedef void (*TFinalizer)(T value);
+  typedef void (*TSetter)(TConfiguration *inst, T val, T *dest);
+  typedef void (*TFinalizer)(T val);
 
  private:
   /*!
@@ -87,11 +87,6 @@
   const char *configName;
 
   /*!
-   * \brief Configuration value.
-   */
-  T value;
-
-  /*!
    * \brief Setter of this configuration.
    *        If this value is NULL, configuration will be stored directly.
    */
@@ -106,6 +101,12 @@
    */
   TFinalizer finalizer;
 
+ protected:
+  /*!
+   * \brief Configuration value.
+   */
+  T value;
+
  public:
   TConfigElement(TConfiguration *cnf, const char *name, T initVal,
                  TSetter sttr = NULL, TFinalizer fnlzr = NULL) {
@@ -121,11 +122,9 @@
   TConfigElement(const TConfigElement<T, V> &src) {
     config = src.config;
     configName = strdup(src.configName);
-    value = (T)0;
     setter = src.setter;
     finalizer = src.finalizer;
-
-    set(src.value);
+    value = src.value;
   }
 
   virtual ~TConfigElement() {
@@ -170,10 +169,26 @@
 typedef TConfigElement<bool, BOOLEAN> TBooleanConfig;
 typedef TConfigElement<int, INTEGER> TIntConfig;
 typedef TConfigElement<jlong, LONG> TLongConfig;
-typedef TConfigElement<char *, STRING> TStringConfig;
 typedef TConfigElement<TLogLevel, LOGLEVEL> TLogLevelConfig;
 typedef TConfigElement<TRankOrder, RANKORDER> TRankOrderConfig;
 
+class TStringConfig : public TConfigElement<char *, STRING> {
+
+  public:
+    TStringConfig(TConfiguration *cnf, const char *name, char *initVal,
+                  TSetter sttr = NULL, TFinalizer fnlzr = NULL) :
+              TConfigElement<char *, STRING>(cnf, name, initVal, sttr, fnlzr) {}
+
+    /* Override copy constructor. */
+    TStringConfig(const TStringConfig &src) :
+                           TConfigElement<char *, STRING>(src) {
+      value = (src.value == NULL) ? NULL : strdup(src.value);
+    }
+
+    virtual ~TStringConfig() {}
+};
+
+
 /*!
  * \brief Configuration holder.
  *        This class contains current configuration value.
--- a/agent/src/heapstats-engines/heapstatsMBean.cpp	Fri Feb 26 21:58:02 2016 +0900
+++ b/agent/src/heapstats-engines/heapstatsMBean.cpp	Sat Mar 05 22:45:53 2016 +0900
@@ -522,74 +522,86 @@
 
   for (std::list<TConfigElementSuper *>::iterator itr = configs.begin();
        itr != configs.end(); itr++) {
-    if (strcmp(opt, (*itr)->getConfigName()) == 0) {
-      switch ((*itr)->getConfigDataType()) {
-        case BOOLEAN:
-          if (env->IsAssignableFrom(valueCls, boolCls)) {
-            jboolean newval = env->CallBooleanMethod(value, boolValue);
-            ((TBooleanConfig *)*itr)->set((bool)newval);
-          } else {
-            raiseException(env, "java/lang/ClassCastException",
-                           "Cannot convert new configuration to Boolean.");
-          }
-          break;
-        case INTEGER:
-          if (env->IsAssignableFrom(valueCls, integerCls)) {
-            jint newval = env->CallIntMethod(value, intValue);
-            ((TIntConfig *)*itr)->set(newval);
-          } else {
-            raiseException(env, "java/lang/ClassCastException",
-                           "Cannot convert new configuration to Integer.");
-          }
-          break;
-        case LONG:
-          if (env->IsAssignableFrom(valueCls, longCls)) {
-            jlong newval = env->CallLongMethod(value, longValue);
-            ((TLongConfig *)*itr)->set(newval);
-          } else {
-            raiseException(env, "java/lang/ClassCastException",
-                           "Cannot convert new configuration to Long.");
-          }
-          break;
-        case LOGLEVEL:
-          for (int Cnt = 0; Cnt < env->GetArrayLength(logLevelArray); Cnt++) {
-            if (env->IsSameObject(
-                    env->GetObjectArrayElement(logLevelArray, Cnt), value)) {
-              ((TLogLevelConfig *)*itr)->set((TLogLevel)Cnt);
-              break;
+    try{
+      if (strcmp(opt, (*itr)->getConfigName()) == 0) {
+        switch ((*itr)->getConfigDataType()) {
+          case BOOLEAN:
+            if (env->IsAssignableFrom(valueCls, boolCls)) {
+              jboolean newval = env->CallBooleanMethod(value, boolValue);
+              ((TBooleanConfig *)*itr)->set((bool)newval);
+            } else {
+              raiseException(env, "java/lang/ClassCastException",
+                             "Cannot convert new configuration to Boolean.");
+            }
+            break;
+          case INTEGER:
+            if (env->IsAssignableFrom(valueCls, integerCls)) {
+              jint newval = env->CallIntMethod(value, intValue);
+              ((TIntConfig *)*itr)->set(newval);
+            } else {
+              raiseException(env, "java/lang/ClassCastException",
+                             "Cannot convert new configuration to Integer.");
+            }
+            break;
+          case LONG:
+            if (env->IsAssignableFrom(valueCls, longCls)) {
+              jlong newval = env->CallLongMethod(value, longValue);
+              ((TLongConfig *)*itr)->set(newval);
+            } else {
+              raiseException(env, "java/lang/ClassCastException",
+                             "Cannot convert new configuration to Long.");
+            }
+            break;
+          case LOGLEVEL:
+            for (int Cnt = 0; Cnt < env->GetArrayLength(logLevelArray); Cnt++) {
+              if (env->IsSameObject(
+                      env->GetObjectArrayElement(logLevelArray, Cnt), value)) {
+                ((TLogLevelConfig *)*itr)->set((TLogLevel)Cnt);
+                break;
+              }
             }
-          }
-          break;
-        case RANKORDER:
-          for (int Cnt = 0; Cnt < env->GetArrayLength(rankOrderArray); Cnt++) {
-            if (env->IsSameObject(
-                    env->GetObjectArrayElement(rankOrderArray, Cnt), value)) {
-              ((TRankOrderConfig *)*itr)->set((TRankOrder)Cnt);
-              break;
+            break;
+          case RANKORDER:
+            for (int Cnt = 0;
+                 Cnt < env->GetArrayLength(rankOrderArray); Cnt++) {
+              if (env->IsSameObject(
+                      env->GetObjectArrayElement(rankOrderArray, Cnt), value)) {
+                ((TRankOrderConfig *)*itr)->set((TRankOrder)Cnt);
+                break;
+              }
             }
-          }
-          break;
-        default:  // String
+            break;
+          default:  // String
+
+            if (value == NULL) {
+              ((TStringConfig *)*itr)->set(NULL);
+            } else if (env->IsAssignableFrom(
+                           valueCls, env->FindClass("java/lang/String"))) {
+              char *val = (char *)env->GetStringUTFChars((jstring)value, NULL);
 
-          if (value == NULL) {
-            ((TStringConfig *)*itr)->set(NULL);
-          } else if (env->IsAssignableFrom(
-                         valueCls, env->FindClass("java/lang/String"))) {
-            char *val = (char *)env->GetStringUTFChars((jstring)value, NULL);
+              if (val == NULL) {
+                raiseException(env, "java/lang/RuntimeException",
+                               "Cannot get string in JNI");
+              } else {
+                ((TStringConfig *)*itr)->set(val);
+                env->ReleaseStringUTFChars((jstring)value, val);
+              }
 
-            if (val == NULL) {
-              raiseException(env, "java/lang/RuntimeException",
-                             "Cannot get string in JNI");
             } else {
-              ((TStringConfig *)*itr)->set(val);
-              env->ReleaseStringUTFChars((jstring)value, val);
+              raiseException(env, "java/lang/RuntimeException",
+                             "Cannot support this configuration type.");
             }
+        }
+      }
+    } catch (const char *msg) {
+      const char *const_msg = " cannot be set new value: ";
+      size_t msg_len = strlen(opt) + strlen(const_msg) + strlen(msg);
+      char *exception_msg = (char *)malloc(msg_len);
+      sprintf(exception_msg, "%s%s%s", opt, const_msg, msg);
 
-          } else {
-            raiseException(env, "java/lang/RuntimeException",
-                           "Cannot support this configuration type.");
-          }
-      }
+      raiseException(env, "java/lang/IllegalArgumentException", exception_msg);
+
+      free(exception_msg);
     }
   }