8132725: Memory leak in Arguments::add_property function
Summary: Logic in add_property was rewritten to avoid memory leak
Reviewed-by: iklam, coleenp
--- a/hotspot/src/share/vm/prims/jvmtiEnv.cpp Fri Aug 28 11:10:57 2015 +0200
+++ b/hotspot/src/share/vm/prims/jvmtiEnv.cpp Fri Aug 28 17:32:31 2015 +0300
@@ -3482,7 +3482,7 @@
for (SystemProperty* p = Arguments::system_properties(); p != NULL; p = p->next()) {
if (strcmp(property, p->key()) == 0) {
- if (p->set_value((char *)value_ptr)) {
+ if (p->set_value(value_ptr)) {
err = JVMTI_ERROR_NONE;
}
}
--- a/hotspot/src/share/vm/runtime/arguments.cpp Fri Aug 28 11:10:57 2015 +0200
+++ b/hotspot/src/share/vm/runtime/arguments.cpp Fri Aug 28 17:32:31 2015 +0300
@@ -983,53 +983,61 @@
bool Arguments::add_property(const char* prop) {
const char* eq = strchr(prop, '=');
- char* key;
- // ns must be static--its address may be stored in a SystemProperty object.
- const static char ns[1] = {0};
- char* value = (char *)ns;
-
- size_t key_len = (eq == NULL) ? strlen(prop) : (eq - prop);
- key = AllocateHeap(key_len + 1, mtInternal);
- strncpy(key, prop, key_len);
- key[key_len] = '\0';
-
- if (eq != NULL) {
- size_t value_len = strlen(prop) - key_len - 1;
- value = AllocateHeap(value_len + 1, mtInternal);
- strncpy(value, &prop[key_len + 1], value_len + 1);
+ const char* key;
+ const char* value = "";
+
+ if (eq == NULL) {
+ // property doesn't have a value, thus use passed string
+ key = prop;
+ } else {
+ // property have a value, thus extract it and save to the
+ // allocated string
+ size_t key_len = eq - prop;
+ char* tmp_key = AllocateHeap(key_len + 1, mtInternal);
+
+ strncpy(tmp_key, prop, key_len);
+ tmp_key[key_len] = '\0';
+ key = tmp_key;
+
+ value = &prop[key_len + 1];
}
if (strcmp(key, "java.compiler") == 0) {
process_java_compiler_argument(value);
- FreeHeap(key);
- if (eq != NULL) {
- FreeHeap(value);
- }
- return true;
- } else if (strcmp(key, "sun.java.command") == 0) {
- _java_command = value;
-
// Record value in Arguments, but let it get passed to Java.
} else if (strcmp(key, "sun.java.launcher.is_altjvm") == 0 ||
strcmp(key, "sun.java.launcher.pid") == 0) {
// sun.java.launcher.is_altjvm and sun.java.launcher.pid property are
// private and are processed in process_sun_java_launcher_properties();
// the sun.java.launcher property is passed on to the java application
- FreeHeap(key);
- if (eq != NULL) {
- FreeHeap(value);
- }
- return true;
- } else if (strcmp(key, "java.vendor.url.bug") == 0) {
- // save it in _java_vendor_url_bug, so JVM fatal error handler can access
- // its value without going through the property list or making a Java call.
- _java_vendor_url_bug = value;
} else if (strcmp(key, "sun.boot.library.path") == 0) {
PropertyList_unique_add(&_system_properties, key, value, true);
- return true;
+ } else {
+ if (strcmp(key, "sun.java.command") == 0) {
+ if (_java_command != NULL) {
+ os::free(_java_command);
+ }
+ _java_command = os::strdup_check_oom(value, mtInternal);
+ } else if (strcmp(key, "java.vendor.url.bug") == 0) {
+ if (_java_vendor_url_bug != DEFAULT_VENDOR_URL_BUG) {
+ assert(_java_vendor_url_bug != NULL, "_java_vendor_url_bug is NULL");
+ os::free((void *)_java_vendor_url_bug);
+ }
+ // save it in _java_vendor_url_bug, so JVM fatal error handler can access
+ // its value without going through the property list or making a Java call.
+ _java_vendor_url_bug = os::strdup_check_oom(value, mtInternal);
+ }
+
+ // Create new property and add at the end of the list
+ PropertyList_unique_add(&_system_properties, key, value);
}
- // Create new property and add at the end of the list
- PropertyList_unique_add(&_system_properties, key, value);
+
+ if (key != prop) {
+ // SystemProperty copy passed value, thus free previously allocated
+ // memory
+ FreeHeap((void *)key);
+ }
+
return true;
}
@@ -1046,7 +1054,7 @@
// Ensure Agent_OnLoad has the correct initial values.
// This may not be the final mode; mode may change later in onload phase.
PropertyList_unique_add(&_system_properties, "java.vm.info",
- (char*)VM_Version::vm_info_string(), false);
+ VM_Version::vm_info_string(), false);
UseInterpreter = true;
UseCompiler = true;
@@ -1858,7 +1866,7 @@
}
// Aggressive optimization flags -XX:+AggressiveOpts
-void Arguments::set_aggressive_opts_flags() {
+jint Arguments::set_aggressive_opts_flags() {
#ifdef COMPILER2
if (AggressiveUnboxing) {
if (FLAG_IS_DEFAULT(EliminateAutoBox)) {
@@ -1885,7 +1893,9 @@
// Feed the cache size setting into the JDK
char buffer[1024];
sprintf(buffer, "java.lang.Integer.IntegerCache.high=" INTX_FORMAT, AutoBoxCacheMax);
- add_property(buffer);
+ if (!add_property(buffer)) {
+ return JNI_ENOMEM;
+ }
}
if (AggressiveOpts && FLAG_IS_DEFAULT(BiasedLockingStartupDelay)) {
FLAG_SET_DEFAULT(BiasedLockingStartupDelay, 500);
@@ -1898,12 +1908,14 @@
// FLAG_SET_DEFAULT(EliminateZeroing, true);
// }
}
+
+ return JNI_OK;
}
//===========================================================================================================
// Parsing of java.compiler property
-void Arguments::process_java_compiler_argument(char* arg) {
+void Arguments::process_java_compiler_argument(const char* arg) {
// For backwards compatibility, Djava.compiler=NONE or ""
// causes us to switch to -Xint mode UNLESS -Xdebug
// is also specified.
@@ -3870,7 +3882,10 @@
set_bytecode_flags();
// Set flags if Aggressive optimization flags (-XX:+AggressiveOpts) enabled
- set_aggressive_opts_flags();
+ jint code = set_aggressive_opts_flags();
+ if (code != JNI_OK) {
+ return code;
+ }
// Turn off biased locking for locking debug mode flags,
// which are subtly different from each other but neither works with
@@ -4036,7 +4051,7 @@
}
}
-void Arguments::PropertyList_add(SystemProperty** plist, const char* k, char* v) {
+void Arguments::PropertyList_add(SystemProperty** plist, const char* k, const char* v) {
if (plist == NULL)
return;
@@ -4049,7 +4064,7 @@
}
// This add maintains unique property key in the list.
-void Arguments::PropertyList_unique_add(SystemProperty** plist, const char* k, char* v, jboolean append) {
+void Arguments::PropertyList_unique_add(SystemProperty** plist, const char* k, const char* v, jboolean append) {
if (plist == NULL)
return;
--- a/hotspot/src/share/vm/runtime/arguments.hpp Fri Aug 28 11:10:57 2015 +0200
+++ b/hotspot/src/share/vm/runtime/arguments.hpp Fri Aug 28 17:32:31 2015 +0300
@@ -60,7 +60,7 @@
char* value() const { return _value; }
SystemProperty* next() const { return _next; }
void set_next(SystemProperty* next) { _next = next; }
- bool set_value(char *value) {
+ bool set_value(const char *value) {
if (writeable()) {
if (_value != NULL) {
FreeHeap(_value);
@@ -364,14 +364,14 @@
static bool add_property(const char* prop);
// Aggressive optimization flags.
- static void set_aggressive_opts_flags();
+ static jint set_aggressive_opts_flags();
// Argument parsing
static void do_pd_flag_adjustments();
static bool parse_argument(const char* arg, Flag::Flags origin);
static bool process_argument(const char* arg, jboolean ignore_unrecognized, Flag::Flags origin);
static void process_java_launcher_argument(const char*, void*);
- static void process_java_compiler_argument(char* arg);
+ static void process_java_compiler_argument(const char* arg);
static jint parse_options_environment_variable(const char* name, ScopedVMInitArgs* vm_args);
static jint parse_java_tool_options_environment_variable(ScopedVMInitArgs* vm_args);
static jint parse_java_options_environment_variable(ScopedVMInitArgs* vm_args);
@@ -561,22 +561,22 @@
// Property List manipulation
static void PropertyList_add(SystemProperty *element);
static void PropertyList_add(SystemProperty** plist, SystemProperty *element);
- static void PropertyList_add(SystemProperty** plist, const char* k, char* v);
- static void PropertyList_unique_add(SystemProperty** plist, const char* k, char* v) {
+ static void PropertyList_add(SystemProperty** plist, const char* k, const char* v);
+ static void PropertyList_unique_add(SystemProperty** plist, const char* k, const char* v) {
PropertyList_unique_add(plist, k, v, false);
}
- static void PropertyList_unique_add(SystemProperty** plist, const char* k, char* v, jboolean append);
+ static void PropertyList_unique_add(SystemProperty** plist, const char* k, const char* v, jboolean append);
static const char* PropertyList_get_value(SystemProperty* plist, const char* key);
static int PropertyList_count(SystemProperty* pl);
static const char* PropertyList_get_key_at(SystemProperty* pl,int index);
static char* PropertyList_get_value_at(SystemProperty* pl,int index);
// Miscellaneous System property value getter and setters.
- static void set_dll_dir(char *value) { _sun_boot_library_path->set_value(value); }
- static void set_java_home(char *value) { _java_home->set_value(value); }
- static void set_library_path(char *value) { _java_library_path->set_value(value); }
+ static void set_dll_dir(const char *value) { _sun_boot_library_path->set_value(value); }
+ static void set_java_home(const char *value) { _java_home->set_value(value); }
+ static void set_library_path(const char *value) { _java_library_path->set_value(value); }
static void set_ext_dirs(char *value) { _ext_dirs = os::strdup_check_oom(value); }
- static void set_sysclasspath(char *value) { _sun_boot_class_path->set_value(value); }
+ static void set_sysclasspath(const char *value) { _sun_boot_class_path->set_value(value); }
static void append_sysclasspath(const char *value) { _sun_boot_class_path->append_value(value); }
static char* get_java_home() { return _java_home->value(); }