Looks good.
Thanks,
David
On 29/01/2015 8:29 PM, Dmitry Samersoff wrote:
David,
Fixed in place (press shift-reload)
http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.04/
-Dmitry
On 2015-01-29 10:26, David Holmes wrote:
On 29/01/2015 12:00 AM, Dmitry Samersoff wrote:
David,
Don't you need to execute the END_WITH_LOCAL_REFS before you can return?
done.
http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.04/
Not quite how I envisaged the fix:
JNI_FUNC_PTR(env,PopLocalFrame)(env, NULL); // END_WITH_LOCAL_REFS
inlining the macro defeats the purpose the macro to some extent - if it
were changed then this code would not get the change. I was thinking
more along the lines of:
WITH_LOCAL_REFS(env, 1) {
char *utf;
utf = (char *)JNI_FUNC_PTR(env,GetStringUTFChars)(env, string, NULL);
if (!(*env)->ExceptionCheck(env)) {
(void)outStream_writeString(out, utf);
JNI_FUNC_PTR(env,ReleaseStringUTFChars)(env, string, utf);
}
} END_WITH_LOCAL_REFS(env);
David
-Dmitry
On 2015-01-28 10:24, David Holmes wrote:
Hi Dmitry,
Sorry this slipped through the cracks.
On 21/01/2015 11:05 PM, Dmitry Samersoff wrote:
David,
Please, take a look at updated webrev
http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.03/
src/jdk.jdwp.agent/share/native/libjdwp/StringReferenceImpl.c.
Don't you need to execute the END_WITH_LOCAL_REFS before you can return?
Otherwise seems okay.
Thanks,
David
-Dmitry
On 2014-10-16 16:07, David Holmes wrote:
Hi Dmitry,
On 16/10/2014 8:08 PM, Dmitry Samersoff wrote:
David,
Changed. Thank you for review!
please, see:
http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.02/
102 if (weakRef == NULL || (*env)->ExceptionCheck(env)) {
Isn't the only time it will return NULL when an exception occurs?
Conversely if an exception occurs then it must return NULL - so the
exception check seems redundant.
But this also suggests you need similar logic at:
182 weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env,
node->ref);
456 lref = JNI_FUNC_PTR(env,NewLocalRef)(env,
node->ref);
Or more generally any JNI call from JVMTI should be wrapped in a way
that checks for exceptions and clears them.
David
-Dmitry
On 2014-10-16 04:24, David Holmes wrote:
On 16/10/2014 12:33 AM, Dmitry Samersoff wrote:
David,
Sorry, copied wrong function!
I mean this call
weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, ref);
that can post OutOfMemoryError
Okay, so shouldn't that be where the exception is cleared:
/* Create weak reference to make sure we have a reference */
weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, ref);
if (weakRef == NULL) {
+ // < clear exception here >
jvmtiDeallocate(node);
return NULL;
}
Thanks,
David
-----
commonRef_refToID() ->
createNode(JNIEnv *env, jobject ref) ->
weakRef = JNI_FUNC_PTR(env,NewWeakGlobalRef)(env, ref);
-Dmitry
On 2014-10-15 16:21, David Holmes wrote:
On 15/10/2014 8:39 PM, Dmitry Samersoff wrote:
On 2014-10-15 14:27, David Holmes wrote:
On 15/10/2014 8:08 PM, Dmitry Samersoff wrote:
Please review the fix:
http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.01/
Added missed exception checks.
src/jdk.jdwp.agent/share/native/libjdwp/outStream.c
What is potentially posting the exception?
JvmtiEnv::GetTag(jobject object, jlong* tag_ptr) called from
commonRef_refToID()
You mean this call:
error =
JVMTI_FUNC_PTR(gdata->jvmti,GetTag)(gdata->jvmti, ref,
&tag);
x
in findNodeByRef which is called by commonRef_refToID? JVM TI
doesn't
post exceptions.
"JVM TI functions never throw exceptions; error conditions are
communicated via the function return value. Any existing exception
state
is preserved across a call to a JVM TI function."
http://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html
David
-Dmitry
