On 4/20/2016 6:06 AM, David Holmes wrote:
On 20/04/2016 10:58 PM, Kumar Srinivasan wrote:

Hello,

One more thing, what about a launcher regression test ?

What kind of regression test? I've manually visually inspected the desired behaviour but a test for it is very problematic. AFAIK there are no tests for setting the native thread name.

Isn't there a management API or something to enumerate the threads ?
I am more worried about some seemingly innocuous launcher change
causing a regression.

Well if it is hard then the jbs must be labelled so, noreg-hard.

Kumar


David

Thanks
Kumar


On 4/19/2016 1:32 PM, David Holmes wrote:
On 20/04/2016 3:00 AM, Kumar Srinivasan wrote:
Hi David,

  493     /* Set native thread name. */
  494     SetNativeThreadName(env, "main");
  495
  496     /* Invoke main method. */
  497     (*env)->CallStaticVoidMethod(env, mainClass, mainID,
mainArgs);

Supposing an exception is thrown in 494 (a very remote case),
will we not re-enter the VM in 497 with an exception ?

Yes as I said:

1712     NULL_CHECK(nameString = (*env)->NewStringUTF(env, name));
1713     (*env)->CallVoidMethod(env, currentThread, setNativeNameID,
1714                            nameString, JNI_TRUE);

As above NULL_CHECK is fine here, but we should check for and clear
any pending exception after CallVoidMethod.

Agreed.

Kumar

Thanks,
David

Kumar

On 19/04/2016 10:54 PM, Gerard Ziemski wrote:

On Apr 19, 2016, at 12:04 AM, David Holmes <david.hol...@oracle.com>
wrote:

On 19/04/2016 9:28 AM, Yasumasa Suenaga wrote:
Hi Gerard,

2016/04/19 3:14 "Gerard Ziemski" <gerard.ziem...@oracle.com
<mailto:gerard.ziem...@oracle.com>>:

hi Yasumasa,

Nice work. I have 2 questions:

========
File: java.c

#1 Shouldn’t we be checking for “(*env)->ExceptionOccurred(env)”
after every single JNI call? In this example instead of NULL_CHECK,
should we be using CHECK_EXCEPTION_NULL_LEAVE macro?

It is not critical if we encounter error at JNI function call
because
we cannot set native thread name only.
So I think that we do not need to leave from launcher process.

I agree we do not need to abort if an exception occurs (and in fact I don't think an exception is even possible from this code), but we should ensure any pending exception is cleared before any futher JNI
calls might be made. Note that NULL_CHECK is already used
extensively throughout the launcher code - so if this usage is wrong
then it is all wrong! More on this code below ...

Isn’t there a risk that:

(*env)->CallVoidMethod(env, currentThread, setNativeNameID,
+                           nameString, JNI_TRUE);

will raise an exception?

In the least, shouldn’t we clear any possible pending exceptions at
the beginning of “SetNativeThreadName“, as you say, but also at the
very end?

I was actually saying at the end, after the call (even though I don't
think any exceptions are possible - except for "async" exceptions of
course). We shouldn't be able to get to the call with any exception
pending as in that case the JNI calls will be returning NULL and we
will exit - again this pattern is used extensively in the launcher.

David


cheers





Reply via email to