Re: JDK 9 RFR of 8030814: Long.parseUnsignedLong should throw exception on too large input

2014-01-09 Thread Paul Sandoz
Hi Brian,

This generally looks good, rather clever trick, i prefer it to using a cache.

I agree with Joe some comments are required as it is not immediately obvious 
how this works.

The additional tests seem adequate (overflow of the overflow), it's easy to go 
overboard e.g. you could test:

  BigInteger b = quotient.multiply(BigInteger.valueOf(radix + N));

for some range of N according to the radix under test. However, it might be 
useful to accurately test boundary conditions.

Paul.

On Jan 3, 2014, at 8:00 PM, Brian Burkhalter brian.burkhal...@oracle.com 
wrote:

 Issue:https://bugs.openjdk.java.net/browse/JDK-8030814
 webrev:   http://cr.openjdk.java.net/~bpb/8030814/webrev.2/
 
 This review request follows from the discussion of last month in this thread:
 
 http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/024031.html
 
 The contributed patch before my minor tweaking of it is here
 
 http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/024110.html
 
 with a detailed explanation of its logic here
 
 http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-December/024136.html
 
 I added to the java/lang/Long/Unsigned JTREG test the case from the issue 
 report as well as some other cases which exercise both sides of the A v B 
 overflow test.
 
 Thanks,
 
 Brian



Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread Tristan Yan

Can someone else give a second review on this.
Thank you
Tristan

On 01/07/2014 07:29 PM, David Holmes wrote:

On 7/01/2014 8:36 PM, Tristan Yan wrote:

Hi David
You're totally right. Sorry I ask you review it again.

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.02/


Looks good now.

Thanks,
David


Thank you very much.
Tristan

On 01/07/2014 05:18 PM, David Holmes wrote:

On 7/01/2014 6:16 PM, Tristan Yan wrote:

Thank you, David
I fixed copyright and change back sleep.
println was intended to be left in. This test was failed with timeout,
printf could help us to detect the value of total_turns_taken and
expected_turns_taken.
Please review it again

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.01/


Comma after 2014 still missing in copyright.

You need to read total_turns_taken.get() once and use that value in
both the println and the == test, so that you print the value you
actually tested.

David


Regards
Tristan

On 01/07/2014 03:10 PM, David Holmes wrote:

Hi Tristan,

On 7/01/2014 4:43 PM, Tristan Yan wrote:

Hi All
Please help to review the code change for JDK-7027502.

http://cr.openjdk.java.net/~tyan/JDK-7027502/

Description:
This test was failed in JPRT test but recently we test with same
binaries run, it doesn't fail any more. The intention for this code
change is bringing this test back to normal test and make this test
robust and more informative.  Change includes
1. Remove this test from ProblemList.
2. Change static member total_turns_taken into a local variable.

Please let me know your comment on this.


   2  * Copyright (c) 2004,2014 Oracle and/or its affiliates. All
rights reserved.

Correct copyright format should have a space before 2014 and a comma
after:

 * Copyright (c) 2004, 2014, Oracle and/or its affiliates. All rights
reserved.

---

Was this println intended to be left in?

 114 System.out.println(total_turns_taken = +
total_turns_taken +
 115 ;expected_turns_taken = +
expected_turns_taken);
 116 if ( total_turns_taken.get() ==
expected_turns_taken ) {


You only want to read total_turns_taken once otherwise you may see
misleading print outs.

---

119 /* Create some monitor contention by sleeping 
with

lock */
 120 if ( default_contention_sleep  0 ) {
 121 System.out.println(Context sleeping, to
create contention);
 122 try {
 123 turn.wait((long)
default_contention_sleep);
 124 } catch (InterruptedException ignore) { }
 125 }

By changing the Thread.sleep to turn.wait you no longer introduce any
contention as the wait() will release the monitor.

David


Thank you.
Tristan








Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread Paul Sandoz

On Jan 9, 2014, at 10:52 AM, Tristan Yan tristan@oracle.com wrote:

 Can someone else give a second review on this.

In a comment the bug you state:

  here total_turns_taken is a static variable, it could affect by other tests

I don't quite know under what test circumstances that can happen, but if so is 
the following also an issue:

  52 private final static TurnChecker turn = new TurnChecker(-1);

?

FWIW an alternative to using an AtomicInteger would be for the main loop to sum 
up thread_turns of each Context, since read/writes are all performed in a 
synchronized block. 

Paul.


hg: jdk8/tl/jdk: 8031187: DoubleStream.count is incorrect for a stream containing Integer.MAX_VALUE elements

2014-01-09 Thread paul . sandoz
Changeset: 630a92015993
Author:psandoz
Date:  2014-01-09 10:52 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/630a92015993

8031187: DoubleStream.count is incorrect for a stream containing  
Integer.MAX_VALUE elements
Reviewed-by: darcy

! src/share/classes/java/util/stream/DoublePipeline.java
! src/share/classes/java/util/stream/IntPipeline.java
+ 
test/java/util/stream/test/org/openjdk/tests/java/util/stream/CountLargeTest.java
+ test/java/util/stream/test/org/openjdk/tests/java/util/stream/CountTest.java



Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread Tristan Yan

Thank you Paul

I change turn to local variable as well.

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.03/ 
http://cr.openjdk.java.net/%7Etyan/JDK-7027502/webrev.03/


I am not sure I understand your second suggestion here,  sum up 
thread_turns of each Context(This is a fixed value) doesn't equal 
total_turns_taken.


Regards

Tristan

On 01/09/2014 06:28 PM, Paul Sandoz wrote:


On Jan 9, 2014, at 10:52 AM, Tristan Yan tristan@oracle.com wrote:


Can someone else give a second review on this.

In a comment the bug you state: here total_turns_taken is a static 
variable, it could affect by other tests I don't quite know under 
what test circumstances that can happen, but if so is the following 
also an issue: 52 private final static TurnChecker turn = new 
TurnChecker(-1); ? FWIW an alternative to using an AtomicInteger would 
be for the main loop to sum up thread_turns of each Context, since 
read/writes are all performed in a synchronized block. Paul.




Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread David Holmes

On 9/01/2014 8:28 PM, Paul Sandoz wrote:


On Jan 9, 2014, at 10:52 AM, Tristan Yan tristan@oracle.com wrote:


Can someone else give a second review on this.


In a comment the bug you state:

   here total_turns_taken is a static variable, it could affect by other tests

I don't quite know under what test circumstances that can happen, but if so is 
the following also an issue:

   52 private final static TurnChecker turn = new TurnChecker(-1);

?

FWIW an alternative to using an AtomicInteger would be for the main loop to sum 
up thread_turns of each Context, since read/writes are all performed in a 
synchronized block.


Now that you mention it ... I had mistakenly thought the switch to an 
AtomicInteger was to ensure the ++ was actually atomic, but as you note 
these updates all take place within a synchronized block that locks the 
same turn instance of TurnChecker.


So now I'm confused by the change. Tristan what was the basis of making 
the change that you did ? If the issue was that concurrent hprof tests 
all used the same Context class in the same VM (how??) then as Paul says 
the TurnChecker instance would also be a problem. Otherwise the change 
makes no real difference to the semantics of the test as far as I can see.


David


Paul.



Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread Paul Sandoz

On Jan 9, 2014, at 12:07 PM, Tristan Yan tristan@oracle.com wrote:

 Thank you Paul
 
 I change turn to local variable as well.
 
 http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.03/
 
 I am not sure I understand your second suggestion here,  sum up thread_turns 
 of each Context(This is a fixed value) doesn't equal total_turns_taken.
 

The suggestion (feel free to ignore it!) was to take the local variable:

 161 int turns_taken   = 0;

and move it to a field on Context. 

Then in the synchronized block of the main loop sum 'em up. e.g.:

  ListContext cs = ;
  ...
  int current_total_turns_taken = cs.stream().mapToInt(c - 
c.turns_taken).sum();

Obviously that is less efficient but it does separate concerns.

Paul.


Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread David Holmes

On 9/01/2014 9:07 PM, Tristan Yan wrote:

Thank you Paul

I change turn to local variable as well.

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.03/
http://cr.openjdk.java.net/%7Etyan/JDK-7027502/webrev.03/


Okay I think I get it now. Both MonitorTest and WaitersTest use the 
Context class, so if both tests run in the same VM the second test will 
see the static total_turns_taken and turn in the state they were left 
from the first test - hence the second test will always fail. The bug 
report suggests making the tests othervm to avoid the problem but 
instead you have changed from using static state to instance state so 
that there is no interference.


But you can't pass a reference to a simple int, for total_turns_taken, 
hence you turned it into the only mutable Integer type: AtomicInteger.


I'm okay with this final form of the change. othervm would have been 
simpler :) These are ugly tests even with your changes.


BTW:

 107 Thread.yield();
 108 Thread.sleep(sleepTime);

The yield() before the sleep is completely pointless.

David
-


I am not sure I understand your second suggestion here,  sum up
thread_turns of each Context(This is a fixed value) doesn't equal
total_turns_taken.

Regards

Tristan

On 01/09/2014 06:28 PM, Paul Sandoz wrote:


On Jan 9, 2014, at 10:52 AM, Tristan Yan tristan@oracle.com wrote:


Can someone else give a second review on this.


In a comment the bug you state: here total_turns_taken is a static
variable, it could affect by other tests I don't quite know under
what test circumstances that can happen, but if so is the following
also an issue: 52 private final static TurnChecker turn = new
TurnChecker(-1); ? FWIW an alternative to using an AtomicInteger would
be for the main loop to sum up thread_turns of each Context, since
read/writes are all performed in a synchronized block. Paul.



Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread David Holmes

On 9/01/2014 9:27 PM, David Holmes wrote:

On 9/01/2014 9:07 PM, Tristan Yan wrote:

Thank you Paul

I change turn to local variable as well.

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.03/
http://cr.openjdk.java.net/%7Etyan/JDK-7027502/webrev.03/


Okay I think I get it now. Both MonitorTest and WaitersTest use the
Context class, so if both tests run in the same VM the second test will
see the static total_turns_taken and turn in the state they were left
from the first test - hence the second test will always fail. The bug
report suggests making the tests othervm to avoid the problem but
instead you have changed from using static state to instance state so
that there is no interference.

But you can't pass a reference to a simple int, for total_turns_taken,
hence you turned it into the only mutable Integer type: AtomicInteger.

I'm okay with this final form of the change. othervm would have been
simpler :) These are ugly tests even with your changes.

BTW:

  107 Thread.yield();
  108 Thread.sleep(sleepTime);

The yield() before the sleep is completely pointless.


Ditto for:

 126 Thread.yield();
 127 Thread.sleep((long)default_contention_sleep);

David
-


David
-


I am not sure I understand your second suggestion here,  sum up
thread_turns of each Context(This is a fixed value) doesn't equal
total_turns_taken.

Regards

Tristan

On 01/09/2014 06:28 PM, Paul Sandoz wrote:


On Jan 9, 2014, at 10:52 AM, Tristan Yan tristan@oracle.com wrote:


Can someone else give a second review on this.


In a comment the bug you state: here total_turns_taken is a static
variable, it could affect by other tests I don't quite know under
what test circumstances that can happen, but if so is the following
also an issue: 52 private final static TurnChecker turn = new
TurnChecker(-1); ? FWIW an alternative to using an AtomicInteger would
be for the main loop to sum up thread_turns of each Context, since
read/writes are all performed in a synchronized block. Paul.



Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread Tristan Yan

Thanks David
I removed those pointless yield, is it okay you can sponsor this for me?

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.04/ 
http://cr.openjdk.java.net/%7Etyan/JDK-7027502/webrev.04/


Regards
Tristan

On 01/09/2014 07:34 PM, David Holmes wrote:

On 9/01/2014 9:27 PM, David Holmes wrote:

On 9/01/2014 9:07 PM, Tristan Yan wrote:

Thank you Paul

I change turn to local variable as well.

http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.03/
http://cr.openjdk.java.net/%7Etyan/JDK-7027502/webrev.03/


Okay I think I get it now. Both MonitorTest and WaitersTest use the
Context class, so if both tests run in the same VM the second test will
see the static total_turns_taken and turn in the state they were left
from the first test - hence the second test will always fail. The bug
report suggests making the tests othervm to avoid the problem but
instead you have changed from using static state to instance state so
that there is no interference.

But you can't pass a reference to a simple int, for total_turns_taken,
hence you turned it into the only mutable Integer type: AtomicInteger.

I'm okay with this final form of the change. othervm would have been
simpler :) These are ugly tests even with your changes.

BTW:

  107 Thread.yield();
  108 Thread.sleep(sleepTime);

The yield() before the sleep is completely pointless.


Ditto for:

 126 Thread.yield();
 127 Thread.sleep((long)default_contention_sleep);

David
-


David
-


I am not sure I understand your second suggestion here,  sum up
thread_turns of each Context(This is a fixed value) doesn't equal
total_turns_taken.

Regards

Tristan

On 01/09/2014 06:28 PM, Paul Sandoz wrote:

On Jan 9, 2014, at 10:52 AM, Tristan Yan tristan@oracle.com 
wrote:



Can someone else give a second review on this.


In a comment the bug you state: here total_turns_taken is a static
variable, it could affect by other tests I don't quite know under
what test circumstances that can happen, but if so is the following
also an issue: 52 private final static TurnChecker turn = new
TurnChecker(-1); ? FWIW an alternative to using an AtomicInteger would
be for the main loop to sum up thread_turns of each Context, since
read/writes are all performed in a synchronized block. Paul.





Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread Alan Bateman

On 09/01/2014 11:27, David Holmes wrote:


Okay I think I get it now. Both MonitorTest and WaitersTest use the 
Context class, so if both tests run in the same VM the second test 
will see the static total_turns_taken and turn in the state they 
were left from the first test - hence the second test will always 
fail. The bug report suggests making the tests othervm to avoid the 
problem but instead you have changed from using static state to 
instance state so that there is no interference. 
I haven't been following this one closely but I thought that jtreg 
created a class loader for each test (irrespective of mode) so I 
wouldn't expect statics to be an issue.


-Alan.


Re: JDK 9 RFR of 8030814: Long.parseUnsignedLong should throw exception on too large input

2014-01-09 Thread Brian Burkhalter

On Jan 9, 2014, at 1:48 AM, Paul Sandoz wrote:

 This generally looks good, rather clever trick, i prefer it to using a cache.
 
 I agree with Joe some comments are required as it is not immediately obvious 
 how this works.

I have some simpler comments almost done being redacted.

 The additional tests seem adequate (overflow of the overflow), it's easy to 
 go overboard e.g. you could test:
 
  BigInteger b = quotient.multiply(BigInteger.valueOf(radix + N));
 
 for some range of N according to the radix under test. However, it might be 
 useful to accurately test boundary conditions.

Working on that also.

Thanks,

Brian

Re: JDK 9 RFR for 8031068: java/util/logging/ParentLoggersTest.java: checkLoggers: getLoggerNames() returned unexpected loggers

2014-01-09 Thread Daniel Fuchs

On 1/8/14 11:37 PM, Chris Hegarty wrote:



On 8 Jan 2014, at 20:04, Mandy Chung mandy.ch...@oracle.com wrote:



On 1/8/2014 11:30 AM, Daniel Fuchs wrote:

http://cr.openjdk.java.net/~dfuchs/webrev_8031068-jdk9/webrev.01/


The fix looks fine.   The WeakReference and System.gc() call was to help 
reproduce the test failure.  I wonder if it's really needed to be retained in 
the test (or better to remove it).


Sounds like the most reasonable solution.


OK - I removed the unnecessary calls.

http://cr.openjdk.java.net/~dfuchs/webrev_8031068-jdk9/webrev.02/

-- daniel



-Chris.


There has been a few test failures due to this eager GC issue running in 
-server -Xcomp mode (I guess they are from the hotspot nightly testing).   It 
might worth to run all logging tests with -server -Xcomp to find out if all 
test failures due to this issue are reported and fix them all together.

Mandy





Re: JDK 9 RFR for 8031068: java/util/logging/ParentLoggersTest.java: checkLoggers: getLoggerNames() returned unexpected loggers

2014-01-09 Thread Mandy Chung

On 1/9/2014 10:18 AM, Daniel Fuchs wrote:

OK - I removed the unnecessary calls.

http://cr.openjdk.java.net/~dfuchs/webrev_8031068-jdk9/webrev.02/


Thumbs up!  Thanks for the update.

You have fixed a few test failures of this issue.  Do you know if there 
is any other logging test that should be fixed?


Mandy



Re: JDK 9 RFR for 8031068: java/util/logging/ParentLoggersTest.java: checkLoggers: getLoggerNames() returned unexpected loggers

2014-01-09 Thread Chris Hegarty


 On 9 Jan 2014, at 18:18, Daniel Fuchs daniel.fu...@oracle.com wrote:
 
 On 1/8/14 11:37 PM, Chris Hegarty wrote:
 
 On 8 Jan 2014, at 20:04, Mandy Chung mandy.ch...@oracle.com wrote:
 
 
 On 1/8/2014 11:30 AM, Daniel Fuchs wrote:
 
 http://cr.openjdk.java.net/~dfuchs/webrev_8031068-jdk9/webrev.01/
 
 The fix looks fine.   The WeakReference and System.gc() call was to help 
 reproduce the test failure.  I wonder if it's really needed to be retained 
 in the test (or better to remove it).
 
 Sounds like the most reasonable solution.
 
 OK - I removed the unnecessary calls.
 
 http://cr.openjdk.java.net/~dfuchs/webrev_8031068-jdk9/webrev.02/

Thanks. Look good to me.

-Chris.

 
 -- daniel
 
 
 -Chris.
 
 There has been a few test failures due to this eager GC issue running in 
 -server -Xcomp mode (I guess they are from the hotspot nightly testing).   
 It might worth to run all logging tests with -server -Xcomp to find out if 
 all test failures due to this issue are reported and fix them all together.
 
 Mandy
 


Re: JDK 9 RFR for 8031068: java/util/logging/ParentLoggersTest.java: checkLoggers: getLoggerNames() returned unexpected loggers

2014-01-09 Thread Daniel Fuchs

On 1/9/14 7:38 PM, Mandy Chung wrote:

On 1/9/2014 10:18 AM, Daniel Fuchs wrote:

OK - I removed the unnecessary calls.

http://cr.openjdk.java.net/~dfuchs/webrev_8031068-jdk9/webrev.02/


Thumbs up!  Thanks for the update.

You have fixed a few test failures of this issue.  Do you know if 
there is any other logging test that should be fixed?


Yeah. That's what of the tests that had been identified by Seán in 
https://bugs.openjdk.java.net/browse/JDK-8029595.
Hopefully there's only one left now - though I may have to review my 
analysis as I did it before discovering that
holding the logger in a local variable was not enough to prevent garbage 
collection...


-- daniel


Mandy





Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread David Holmes

On 9/01/2014 10:14 PM, Alan Bateman wrote:

On 09/01/2014 11:27, David Holmes wrote:


Okay I think I get it now. Both MonitorTest and WaitersTest use the
Context class, so if both tests run in the same VM the second test
will see the static total_turns_taken and turn in the state they
were left from the first test - hence the second test will always
fail. The bug report suggests making the tests othervm to avoid the
problem but instead you have changed from using static state to
instance state so that there is no interference.

I haven't been following this one closely but I thought that jtreg
created a class loader for each test (irrespective of mode) so I
wouldn't expect statics to be an issue.


That aside DemoRun forks off its own JVM to run a given test anyway!

So I don't understand how the proposed fixes could actually be 
addressing the hangs that are occurring. Even if the statics were being 
shared I don't see how that leads to the failure mode in the bug report.


David


-Alan.


Re: RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

2014-01-09 Thread Tristan Yan

Hi David
I wasn't able to reproduce this failure either in local or in our same 
binaries running(This is a continuous running with same JDK binaries). 
So intention for this code change is bringing this test back;  add some 
debug info and try to avoid possible issues in this test. I agree this 
code change won't solve the failure happened. But this test was put into 
ProblemList two years ago better move for this is move out it from 
ProblemList and trace down the issue in our normal nightly.

Thank you
Tristan

On 01/10/2014 06:35 AM, David Holmes wrote:

On 9/01/2014 10:14 PM, Alan Bateman wrote:

On 09/01/2014 11:27, David Holmes wrote:


Okay I think I get it now. Both MonitorTest and WaitersTest use the
Context class, so if both tests run in the same VM the second test
will see the static total_turns_taken and turn in the state they
were left from the first test - hence the second test will always
fail. The bug report suggests making the tests othervm to avoid the
problem but instead you have changed from using static state to
instance state so that there is no interference.

I haven't been following this one closely but I thought that jtreg
created a class loader for each test (irrespective of mode) so I
wouldn't expect statics to be an issue.


That aside DemoRun forks off its own JVM to run a given test anyway!

So I don't understand how the proposed fixes could actually be 
addressing the hangs that are occurring. Even if the statics were 
being shared I don't see how that leads to the failure mode in the bug 
report.


David


-Alan.




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

2014-01-09 Thread srikalyan chandrashekar
David/Peter you are right, the logs trace came from passed run, i am 
trying to simulate the failure and get the logs for failed runs(2000+ 
runs done and still no failure), will get back to you once i have the 
data from failed run. Sorry for the confusion.


---
Thanks
kalyan

On 01/08/2014 11:22 PM, David Holmes wrote:

Thanks Peter.

Kalyan: Can you confirm, as Peter asked, that the TraceExceptions 
output came from a failed run?


AFAICS the Trace info is printed after each bytecode where there is a 
pending exception - though I'm not 100% sure on the printing within 
the VM runtime. Based on that I think we see the Trace output in run() 
at the point where wait() returns, so it may well be caught after that 
- in which case this was not a failing run.


I also can't reproduce the problem :(

David

On 8/01/2014 10:34 PM, Peter Levart wrote:

On 01/08/2014 07:30 AM, David Holmes wrote:

On 8/01/2014 4:19 PM, David Holmes wrote:

On 8/01/2014 7:33 AM, srikalyan chandrashekar wrote:
Hi David, TraceExceptions with fastdebug build produced some nice 
trace
http://cr.openjdk.java.net/%7Esrikchan/OOME_exception_trace.log 
. The
native method wait(long) is where the OOME if being thrown, the 
deepest

call is in

src/share/vm/gc_interface/collectedHeap.inline.hpp, line 157


Yes but it is the caller that is of interest:

Exception a 'java/lang/OutOfMemoryError' (0xd6a01840)
thrown
[/HUDSON/workspace/8-2-build-linux-amd64/jdk8/1317/hotspot/src/share/vm/runtime/objectMonitor.cpp, 



line 1649]
for thread 0x7f78c40d2800
Exception a 'java/lang/OutOfMemoryError' (0xd6a01840)
  thrown in interpreter method {method} {0x7f78b4800ae0} 'wait'
'(J)V' in 'java/lang/Object'
  at bci 0 for thread 0x7f78c40d2800

The ReferenceHandler thread gets the OOME trying to allocate the
InterruptedException.


However we already have a catch block around the wait() so how is this
OOME getting through? A bug in exception handling in the interpreter ??



Might be. And it may have something to do with the fact that the
Thread.run() method is the 1st call frame on the thread's stack (seems
like corner case). The last few meaningful TraceExceptions records are:


Exception a 'java/lang/OutOfMemoryError' (0xd6a01840)
thrown
[/HUDSON/workspace/8-2-build-linux-amd64/jdk8/1317/hotspot/src/share/vm/gc_interface/collectedHeap.inline.hpp, 


line 157]
for thread 0x7f78c40d2800
Exception a 'java/lang/OutOfMemoryError' (0xd6a01840)
thrown
[/HUDSON/workspace/8-2-build-linux-amd64/jdk8/1317/hotspot/src/share/vm/runtime/objectMonitor.cpp, 


line 1649]
for thread 0x7f78c40d2800
Exception a 'java/lang/OutOfMemoryError' (0xd6a01840)
  thrown in interpreter method {method} {0x7f78b4800ae0} 'wait'
'(J)V' in 'java/lang/Object'
  at bci 0 for thread 0x7f78c40d2800
Exception a 'java/lang/OutOfMemoryError' (0xd6a01840)
  thrown in interpreter method {method} {0x7f78b4800ca8} 'wait'
'()V' in 'java/lang/Object'
  at *bci 2* for thread 0x7f78c40d2800
Exception a 'java/lang/OutOfMemoryError' (0xd6a01840)
  thrown in interpreter method {method} {0x7f78b48d2250} 'run'
'()V' in 'java/lang/ref/Reference$ReferenceHandler'
  at *bci 36* for thread 0x7f78c40d2800


Here's the relevant bytecodes:


public class java.lang.Object

   public final void wait() throws java.lang.InterruptedException;
 descriptor: ()V
 flags: ACC_PUBLIC, ACC_FINAL
 Code:
   stack=3, locals=1, args_size=1
  0: aload_0
  1: lconst_0
* 2: invokevirtual #73 // Method wait:(J)V*
  5: return
   LineNumberTable:
 line 502: 0
 line 503: 5
 Exceptions:
   throws java.lang.InterruptedException


class java.lang.ref.Reference$ReferenceHandler extends java.lang.Thread

   public void run();
 descriptor: ()V
 flags: ACC_PUBLIC
 Code:
   stack=2, locals=5, args_size=1
  0: invokestatic  #62 // Method
java/lang/ref/Reference.access$100:()Ljava/lang/ref/Reference$Lock;
  3: dup
  4: astore_2
  5: monitorenter
  6: invokestatic  #61 // Method
java/lang/ref/Reference.access$200:()Ljava/lang/ref/Reference;
  9: ifnull33
 12: invokestatic  #61 // Method
java/lang/ref/Reference.access$200:()Ljava/lang/ref/Reference;
 15: astore_1
 16: aload_1
 17: invokestatic  #64 // Method
java/lang/ref/Reference.access$300:(Ljava/lang/ref/Reference;)Ljava/lang/ref/Reference; 


 20: invokestatic  #63 // Method
java/lang/ref/Reference.access$202:(Ljava/lang/ref/Reference;)Ljava/lang/ref/Reference; 


 23: pop
 24: aload_1
 25: aconst_null
 26: invokestatic  #65 // Method
java/lang/ref/Reference.access$302:(Ljava/lang/ref/Reference;Ljava/lang/ref/Reference;)Ljava/lang/ref/Reference; 


 29: pop

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

2014-01-09 Thread Dan Xu

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: RFR: JDK-8029007, Check src/share/native/sun/misc code for JNI pending exceptions

2014-01-09 Thread Chris Hegarty
Looks ok to me.

I presume getThreadStateValues is simply no longer needed.

-Chris.

 On 10 Jan 2014, at 06:31, Dan Xu dan...@oracle.com 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