On 1/29/15 12:06 AM, [email protected] wrote:
Hi Dmitry,


It looks good in general.

In fact, I've reviewed the webrev.3 (but replied on the wrong email with webrev.2):
http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.03/


Thanks,
Serguei

src/jdk.jdwp.agent/share/native/libjdwp/StringReferenceImpl.c

I agree with David, the fix needs the "END_WITH_LOCAL_REFS(env);" before the line 50.

src/jdk.jdwp.agent/share/native/libjdwp/commonRef.c
  159          * It never throws OOM

Minor: Could you, please, add a dot at the end of the comment statement?

No need to re-review after the above is fixed.

Thanks,
Serguei


On 10/16/14 3:08 AM, Dmitry Samersoff wrote:
David,

Changed. Thank you for review!

please, see:

http://cr.openjdk.java.net/~dsamersoff/JDK-8030708/webrev.02/

-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




Reply via email to