Jaroslav, Looks good for me.
Shouldn't we abort the loop after the first error? -Dmitry On 2014-07-25 17:04, Jaroslav Bachorik wrote: > On 07/25/2014 01:40 PM, Dmitry Samersoff wrote: >> Jaroslav, >> >> readProviderData already use CHECK >> >> So it might be better to: >> >> 1. change readProviderData to return 0 on error and 1 on success. >> >> 2. add check for exception to ll 201 and abort the loop if at least >> one of providers is not read. > > Ok, here is another attempt, now without introducing a new function. > > http://cr.openjdk.java.net/~jbachorik/8030115/webrev.01 > > -JB- > >> >> >> -Dmitry >> >> >> On 2014-07-24 15:07, Jaroslav Bachorik wrote: >>> Please, review the change for the fix of the following problem >>> >>> In jdk/src/share/native/sun/tracing/dtrace/JVM.c the following code is >>> invoked in loop >>> >>> 198 for (i = 0; i < num_providers; ++i) { >>> 199 JVM_DTraceProvider* p = &(jvm_providers[i]); >>> 200 jobject provider = (*env)->GetObjectArrayElement( >>> 201 env, providers, i); >>> 202 readProviderData(env, provider, p); >>> 203 } >>> >>> The first problem is that GetGetObjectArrayElement() call on L200 may >>> throw an exception which is not checked subsequently. On L202 the call >>> to readProviderData() can also raise a Java exception without >>> appropriate after-check. When getting back to the beginning of the loop >>> GetObjectArrayElement() may be called with a pending exception which is >>> deemed unsafe. >>> >>> The fix extracts the inner part of the loop into a separate function >>> where the result of each step is properly checked for pending >>> exceptions. If there is a pending exception the loop will be >>> interrupted, resources cleaned and >>> Java_sun_tracing_dtrace_JVM_activate0() function will return 0 with the >>> pending exception recorded in env. >>> >>> >>> Webrev: http://cr.openjdk.java.net/~jbachorik/8030115/webrev.00 >>> >>> Thanks, >>> >>> -JB- >> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
