Serguei, Thanks.
-Dmitry On 2015-01-29 11:10, [email protected] wrote: > > > 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 >>>>>>> >>>>>>> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
