Re: 8031494: [launcher] java launcher should check for JNI Pending exceptions.

2014-01-11 Thread Chris Hegarty
Looks ok to me Kumar.

-Chris.

 On 11 Jan 2014, at 00:55, Kumar Srinivasan kumar.x.sriniva...@oracle.com 
 wrote:
 
 Hi,
 
 Please review fixes for launcher correctness wrt. JNI calls.
 
 http://cr.openjdk.java.net/~ksrini/8031494/webrev.0/
 
 Thanks
 Kumar
 


Re: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-11 Thread Chris Hegarty

 On 10 Jan 2014, at 19:40, Dan Xu dan...@oracle.com wrote:
 
 Thank you for all the feedback. I have updated my changes to use 
 CHECK_EXCEPTION_RETURN and CHECK_EXCEPTION macro recently added into 
 jni_util.h. I also removed else block in function setStaticIntField() in 
 Version.c since (*env)-GetStaticFieldID will throw a same exception if the 
 field cannot be found.
 
 Here is the new webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.01/. 
 Thanks!

Looks good to me Dan.

-Chris.

 
 -Dan
 
 
 On 01/10/2014 10:21 AM, Mike Duigou wrote:
 On Jan 10 2014, at 10:09 , Chris Hegarty chris.hega...@oracle.com wrote:
 
 On 10 Jan 2014, at 18:05, Dan Xu dan...@oracle.com wrote:
 
 Hi Roger,
 
 My macro is a little different from yours, which compares with -1 instead 
 of NULL. I also see CHECK_EXCEPTION macro. Thanks for adding them, which 
 are useful when I cannot decide the pending exception state by just using 
 return values.
 
 As for the style, actually I prefer the (!pointer) to (pointer == NULL) 
 because it is more concise and also make me avoid the typo like (pointer = 
 NULL), which I cannot find from the compilation. Thanks!
 There's always yoda conditions 
 https://en.wikipedia.org/wiki/Yoda_conditions, (NULL == pointer), but that's 
 not likely to make anyone (besides me) happy.
 
 Mike
 
 Not that it matters, but my preference is to == NULL.
 
 -Chris.
 
 -Dan
 
 
 On 01/10/2014 08:40 AM, roger riggs wrote:
 Hi Dan,
 
 Just pushed are macros in jni_util.h to do the same function as your new 
 macros.
 Please update to use the common macros instead of introducing new ones.
 
 Style wise, I would avoid mixing binary operators (!) with pointers.
 it is clearer to compare with NULL.  (The CHECK_NULL macro will do the 
 check and return).
 
 (Not a Reviewer)
 
 Thanks, Roger
 
 
 
 On 1/10/2014 1:31 AM, Dan Xu wrote:
 Hi All,
 
 Please review the fix for JNI pending exception issues reported in 
 jdk-8029007. Thanks!
 
 Webrev: http://cr.openjdk.java.net/~dxu/8029007/webrev.00/
 Bug: https://bugs.openjdk.java.net/browse/JDK-8029007
 
 -Dan
 


Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently

2014-01-11 Thread Peter Levart


On 01/10/2014 10:51 PM, srikalyan chandrashekar wrote:
Hi Peter the version you provided ran indefinitely(i put a 10 minute 
timeout) and the program got interrupted(no error),


Did you run it with or without fastedbug  -XX:+TraceExceptions ? If 
with, it might be that fastdebug and/or -XX:+TraceExceptions changes the 
execution a bit so that we can no longer reproduce the wrong behaviour.


even if there were to be an error you cannot print the string of 
thread to console(these have been attempted earlier).


...it has been attempted to print toString in uncaught exception 
handler. At that time, the heap is still full. I'm printing it after the 
GC has cleared the heap. You can try that it works by commenting out the 
try { and corresponding } catch (OOME x) {} exception handler...


- The test's running on interpreter mode, what i am watching for is 
one error with trace. Without fastdebug build and -XX:+TraceExceptions 
i am able to reproduce failure atleast 5 failures out of 1000 runs but 
with fastdebug+Trace no luck yet(already past few 1000 runs).


It might be interesting to try with fastebug build but without the 
-XX:+TraceExceptions option to see what has an effect on it. It might 
also be interesting to try the modified ReferenceHandler (the one with 
private runImpl() method called from run()) and with normal 
non-fastdebug JDK. This info might be useful when one starts to inspect 
the exception handling code in interpreter...


Regards, Peter



---
Thanks
kalyan

On 01/10/2014 02:57 AM, Peter Levart wrote:

On 01/10/2014 09:31 AM, Peter Levart wrote:
Since we suspect there's something wrong with exception handling in 
interpreter, I devised a hypothetical reproducer that tries to 
simulate ReferenceHandler in many aspects, but doesn't require to be 
a ReferenceHandler:


http://cr.openjdk.java.net/~plevart/misc/OOME/OOMECatchingTest.java

This is designed to run indefinitely and only terminate if/when 
thread dies. Could you run this program in the environment that 
causes the OOMEInReferenceHandler test to fail and see if it 
terminates?


I forgot to mention that in order for this long-running program to 
exhibit interpreter behaviour, it should be run with -Xint option. So 
I suggest:


-Xmx24M -XX:-UseTLAB -Xint

Regards, Peter







Re: 8031494: [launcher] java launcher should check for JNI Pending exceptions.

2014-01-11 Thread Alan Bateman

On 11/01/2014 00:55, Kumar Srinivasan wrote:

Hi,

Please review fixes for launcher correctness wrt. JNI calls.

http://cr.openjdk.java.net/~ksrini/8031494/webrev.0/
Looks okay, the only thing that isn't clear is whether the calls to the 
static methods defined by the launcher helper class can complete with an 
exception or not.


-Alan


Re: Request for approval for bug #8031488

2014-01-11 Thread Alan Bateman

On 10/01/2014 16:28, Iaroslav Savytskyi wrote:

:
There are 3 possibilities:
1) Because it was my own initiative to fix this potential synchronization bug 
and nobody didn’t report it, we can approve my fix and leave this 2 classes 
without synchronized getters. And fix it in MR.
2) Fix it as you propose. But later we will definitely need to change it again 
to volatile for performance reasons.
3) Leave classes with volatile as they are now. And only add SUID to 
TypeConstraintException class.

I know this is a blocker for JDK 8 and I don't want to waste time 
debating the options. So I think #1 or #2 are okay (with a slight prefer 
for #2 because it doesn't require bring back the original bug in 
JAXBException).


#3 is of course the best but from the previous mails then I thought this 
wasn't an option.


So you choose and one of us will help you get this in.

-Alan.