On 31/07/13 14:44, Daniel D. Daugherty wrote:
On 7/31/13 3:45 AM, Kevin Walls wrote:
Hi,
I'd like to get a review on this small change:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8020943
https://jbs.oracle.com/bugs/browse/JDK-8020943
webrev
http://cr.openjdk.java.net/~kevinw/8020943/webrev.00/
src/share/vm/services/gcNotifier.cpp
No comments.
The fix is fine in that you changed the code to call a function that
doesn't leak a JNI handle, but the original JNI handle leak that you
found in java_lang_String::create_from_platform_dependent_str()
remains right?
I think create_from_platform_dependent_str() still needs the call to
"JNIHandles::destroy_local(js)". The next function down in the same
file, java_lang_String::as_platform_dependent_str(), takes care to
call "JNIHandles::destroy_local(js)" for its locally created JNI
handle so there's good precedent for cleaning up such things.
Dan
Hi Dan,
Yes. It was also suggested to me that the JNIHandle would be cleared on
a transition back to Java (which never happens in this case), so
although I don't see many other callers, they may not all see a problem.
In another thread my initial thought was in
create_from_platform_dependent_str itself, to clear the JNIHandle. I
just suggested the simple function-name-change here as all the jni work
is unnecessary for what the gcNotifier needs.
Maybe I should make both changes, including:
$ hg diff ../src/share/vm/classfile/javaClasses.cpp
diff --git a/src/share/vm/classfile/javaClasses.cpp
b/src/share/vm/classfile/javaClasses.cpp
--- a/src/share/vm/classfile/javaClasses.cpp
+++ b/src/share/vm/classfile/javaClasses.cpp
@@ -244,7 +244,10 @@
ThreadToNativeFromVM ttn(thread);
js = (_to_java_string_fn)(thread->jni_environment(), str);
}
- return Handle(THREAD, JNIHandles::resolve(js));
+ // XXX KJW we get a jstring back, but leak it...
+ Handle result = Handle(THREAD, JNIHandles::resolve(js));
+ JNIHandles::destroy_local(js);
+ return result;
}
Thanks
Kevin
It turns out there's a leak in the gc notifier: reproduce by
attaching e.g. JConsole and watching, if there is frequent GC the
number of apparently unowned String objects that can't get collect
continually increases.
In the notifier, the method it calls to create String objects
involves a JNI call that leaves a Handle behind and doesn't get
cleared. There is a simpler method to call, there is no need for
all that work, as we are dealing with a small set of simple strings
in the JVM being converted, to describe the collection type, cause, ...
Thanks
Kevin
