Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-11 Thread Daniel D. Daugherty
On 2/10/14 11:37 PM, David Holmes wrote: Dan - thanks for an awesome analysis once again! You're welcome. Yes mea culpa the code I was looking at was sans Mr Simms most fortuitous change. (I'm suffering from repo overload these days.) No apology needed. It's a good thing that you and Karen

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-10 Thread David Holmes
Dan - thanks for an awesome analysis once again! Yes mea culpa the code I was looking at was sans Mr Simms most fortuitous change. (I'm suffering from repo overload these days.) As per my previous email I could see that a timed-wait, or spurious wakeup, could take us to the potentially proble

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-10 Thread Daniel D. Daugherty
On 2/10/14 1:20 PM, Karen Kinnear wrote: Dan, Thank you so much. My bad - I was looking at a jdk8 repo, not a jdk9 one. No problem... I had the advantage of wanting Mr Simms changes so that I could (more easily) develop the debug code flow hooks that I'm planning to add to the "debug tips and

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-10 Thread Karen Kinnear
Dan, Thank you so much. My bad - I was looking at a jdk8 repo, not a jdk9 one. So I agree that the JDK9 fix as is works. Code change reviewed. For JDK8: I don't believe we were planning to backport this to 8 given risks of changes in this area. I did reach the same conclusion you did, that the

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-10 Thread Daniel D. Daugherty
On 2/9/14 8:37 PM, David Holmes wrote: trimming content ... On 8/02/2014 9:45 AM, Daniel D. Daugherty wrote: On 2/7/14 2:56 PM, Karen Kinnear wrote: 3. Did I read the code correctly that the Thread::SpinAcquire can make a timed park call on the same thread's _ParkEvent? And that this is used t

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-09 Thread David Holmes
trimming content ... On 8/02/2014 9:45 AM, Daniel D. Daugherty wrote: On 2/7/14 2:56 PM, Karen Kinnear wrote: 3. Did I read the code correctly that the Thread::SpinAcquire can make a timed park call on the same thread's _ParkEvent? And that this is used to get on and off the wait queue, i.e. to

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-07 Thread Daniel D. Daugherty
On 2/7/14 2:56 PM, Karen Kinnear wrote: Dan, Greatly impressed. Thanks! And thanks for the review! Very subtle. Since you've worked with this code, that shouldn't be a surprise... :-) We're down to the subtle bugs... :-( Thanks to you and David and Dice and Serguei for a lot of blood,

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-07 Thread Karen Kinnear
Dan, Greatly impressed. Very subtle. Thanks to you and David and Dice and Serguei for a lot of blood, sweat and tears. Apologies that I haven't been following all the details, so a couple of questions. 1. I like the new fix. I totally appreciate the massive documentation in the bug, it really

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-04 Thread Daniel D. Daugherty
On 2/3/14 11:39 PM, David Holmes wrote: On 4/02/2014 12:56 AM, Daniel D. Daugherty wrote: Adding Dave Dice to this thread... On 2/3/14 5:10 AM, David Holmes wrote: Hi Dan, On 2/02/2014 4:38 AM, Daniel D. Daugherty wrote: Greetings, I have a fix ready for the following bug: 8028073 rac

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-03 Thread David Holmes
On 4/02/2014 12:56 AM, Daniel D. Daugherty wrote: Adding Dave Dice to this thread... On 2/3/14 5:10 AM, David Holmes wrote: Hi Dan, On 2/02/2014 4:38 AM, Daniel D. Daugherty wrote: Greetings, I have a fix ready for the following bug: 8028073 race condition in ObjectMonitor implementati

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-03 Thread Daniel D. Daugherty
Thanks for the review Serguei! Dan On 2/3/14 11:36 AM, serguei.spit...@oracle.com wrote: Hi Dan, It looks good to me. Your work on this issue is outstanding. I know what it took to resolve this one. Great job! Thanks, Serguei On 2/1/14 10:38 AM, Daniel D. Daugherty wrote: Greetings, I hav

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-03 Thread serguei.spit...@oracle.com
Hi Dan, It looks good to me. Your work on this issue is outstanding. I know what it took to resolve this one. Great job! Thanks, Serguei On 2/1/14 10:38 AM, Daniel D. Daugherty wrote: Greetings, I have a fix ready for the following bug: 8028073 race condition in ObjectMonitor implementat

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-03 Thread Daniel D. Daugherty
Adding Dave Dice to this thread... On 2/3/14 5:10 AM, David Holmes wrote: Hi Dan, On 2/02/2014 4:38 AM, Daniel D. Daugherty wrote: Greetings, I have a fix ready for the following bug: 8028073 race condition in ObjectMonitor implementation causing deadlocks https://bugs.openjdk.java

Re: code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-03 Thread David Holmes
Hi Dan, On 2/02/2014 4:38 AM, Daniel D. Daugherty wrote: Greetings, I have a fix ready for the following bug: 8028073 race condition in ObjectMonitor implementation causing deadlocks https://bugs.openjdk.java.net/browse/JDK-8028073 On the surface, this is a very simple fix that relo

code review round 0 for ObjectMonitor-JVM/TI hang fix (8028073)

2014-02-01 Thread Daniel D. Daugherty
Greetings, I have a fix ready for the following bug: 8028073 race condition in ObjectMonitor implementation causing deadlocks https://bugs.openjdk.java.net/browse/JDK-8028073 On the surface, this is a very simple fix that relocates a few lines of code, relocates and rewrites the comme