Re: RFR (S) 8039904: dtrace/hotspot/Monitors/Monitors001 fails with "assert(s > 0) failed: Bad size calculated"

2014-04-11 Thread Coleen Phillimore
Thank you Serguei. Coleen On 4/11/14, 3:49 PM, serguei.spit...@oracle.com wrote: The fix looks good. Thanks, Serguei On 4/11/14 7:48 AM, Coleen Phillimore wrote: Summary: Dtrace monitoring uses size before mirror size is set. The refactoring I did for bug https://bugs.openjdk.java.net/brow

Re: RFR (S) 8039904: dtrace/hotspot/Monitors/Monitors001 fails with "assert(s > 0) failed: Bad size calculated"

2014-04-11 Thread serguei.spit...@oracle.com
The fix looks good. Thanks, Serguei On 4/11/14 7:48 AM, Coleen Phillimore wrote: Summary: Dtrace monitoring uses size before mirror size is set. The refactoring I did for bug https://bugs.openjdk.java.net/browse/JDK-8028497 caused this bug. The size of the mirror is filled in by the Instance

Re: RFR: 8038963 com/sun/jdi tests fail because cygwin's ps sometimes misses processes

2014-04-11 Thread Daniel D. Daugherty
> webrev: http://cr.openjdk.java.net/~sla/8038963/webrev.00/ test/com/sun/jdi/ShellScaffold.sh No comments other than thinking about those Win98 fixes brought back painful memories... Thumbs up! Dan On 4/2/14 5:26 AM, Staffan Larsen wrote: We have had many intermittent failures in the

Re: RFR (S) 8039904: dtrace/hotspot/Monitors/Monitors001 fails with "assert(s > 0) failed: Bad size calculated"

2014-04-11 Thread Coleen Phillimore
On 4/11/14, 12:00 PM, Keith McGuigan wrote: Looks good, but why are you not using a newer version of webrev with "next" links?? Thanks Keith. The private copy I had of the "next" link webrev that I had broke for me for some mysterious reason. I filed a bug to see if the "official" version

Re: RFR (S) 8039904: dtrace/hotspot/Monitors/Monitors001 fails with "assert(s > 0) failed: Bad size calculated"

2014-04-11 Thread Keith McGuigan
Looks good, but why are you not using a newer version of webrev with "next" links?? -- - Keith On Fri, Apr 11, 2014 at 10:48 AM, Coleen Phillimore < coleen.phillim...@oracle.com> wrote: > Summary: Dtrace monitoring uses size before mirror size is set. > > The refactoring I did for bug https://b

RFR (S) 8039904: dtrace/hotspot/Monitors/Monitors001 fails with "assert(s > 0) failed: Bad size calculated"

2014-04-11 Thread Coleen Phillimore
Summary: Dtrace monitoring uses size before mirror size is set. The refactoring I did for bug https://bugs.openjdk.java.net/browse/JDK-8028497 caused this bug. The size of the mirror is filled in by the InstanceMirrorKlass allocation but was used for dtrace probes before it in the normal alloc

Re: RFR: 8039173 Propagate errors from Diagnostic Commands as exceptions in the attach framework

2014-04-11 Thread Staffan Larsen
On 11 apr 2014, at 10:36, Ivan Gerasimov wrote: > > On 11.04.2014 11:51, Staffan Larsen wrote: >> Of course…This is getting embarrassing. :) >> >> http://cr.openjdk.java.net/~sla/8039173/webrev.03/ >> > Now it looks fine to me :) Thanks. > Not a reviewer though. > > > There are a few mino

Re: RFR: 8039173 Propagate errors from Diagnostic Commands as exceptions in the attach framework

2014-04-11 Thread Ivan Gerasimov
On 11.04.2014 11:51, Staffan Larsen wrote: Of course…This is getting embarrassing. :) http://cr.openjdk.java.net/~sla/8039173/webrev.03/ Now it looks fine to me :) Not a reviewer though. There are a few minor formatting glitches, but

Re: RFR: 8039173 Propagate errors from Diagnostic Commands as exceptions in the attach framework

2014-04-11 Thread Staffan Larsen
Of course…This is getting embarrassing. :) http://cr.openjdk.java.net/~sla/8039173/webrev.03/ /Staffan On 11 apr 2014, at 09:32, Ivan Gerasimov wrote: > Hi Staffan! > > Isn't the second read in line# 268 erroneous? > > 267 while ((n = sis.read(b)) != -1) { > 268 n = si

Re: RFR: 8039173 Propagate errors from Diagnostic Commands as exceptions in the attach framework

2014-04-11 Thread Ivan Gerasimov
Hi Staffan! Isn't the second read in line# 268 erroneous? 267 while ((n = sis.read(b)) != -1) { 268 n = sis.read(b); Sincerely yours, Ivan On 11.04.2014 11:16, Staffan Larsen wrote: Here is a new version where the 200 char cap is removed: http://cr.openjdk.java.net/~s

Re: RFR: 8039173 Propagate errors from Diagnostic Commands as exceptions in the attach framework

2014-04-11 Thread Staffan Larsen
Here is a new version where the 200 char cap is removed: http://cr.openjdk.java.net/~sla/8039173/webrev.02/ Thanks, /Staffan On 4 apr 2014, at 16:27, Staffan Larsen wrote: > Thanks Ivan. > > I’ve been thinking a bit more about this and discussing with people here. I’m > not sure anymore that