Re: RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

2018-10-10 Thread David Holmes
On 11/10/2018 1:09 PM, JC Beyler wrote: So that's what I thought too but I did this: $ cat Test.java public class Test { public static void main(String[] args) { int res; try { res = 5; } catch (Exception e) { } if (res > 0) { } } } That fails to com

Re: RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

2018-10-10 Thread serguei.spit...@oracle.com
On 10/10/18 19:36, David Holmes wrote: On 11/10/2018 12:19 PM, JC Beyler wrote: Hi Alex, - http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.01/test/lib/jdk/test/lib/apps/LingeredApp.java.udiff.html

Re: RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

2018-10-10 Thread David Holmes
On 11/10/2018 12:19 PM, JC Beyler wrote: Hi Alex, - http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.01/test/lib/jdk/test/lib/apps/LingeredApp.java.udiff.html   -> Wh

Re: RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

2018-10-10 Thread JC Beyler
Hi Alex, - http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.01/test/lib/jdk/test/lib/apps/LingeredApp.java.udiff.html -> Why not make it javadoc like the other methods of the same file (so @return instead of returns and a second * at the start of the comment)? - http://cr.openjdk.java.n

Re: RFR (L) 8211899: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[E-M]

2018-10-10 Thread Alex Menkov
Hi Jc, Overall looks good. one minor note: test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/events/EM06/em06t001/em06t001.cpp: -jclassName = (jstring) (jstring) (jstring) (jstring) (jstring) (jstring) (jstring) (jstring) (jstring) NSK_CPP_STUB3(CallObjectMethod, jni_env, klass, -

Re: RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

2018-10-10 Thread serguei.spit...@oracle.com
Hi Alex, It looks good to me. How did you test it? Thanks, Serguei On 10/10/18 16:25, Alex Menkov wrote: Hi all, please review a fix for https://bugs.openjdk.java.net/browse/JDK-8195703 webrev: http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.01/ I was not able to reproduce the issu

RFR JDK-8195703: BasicJDWPConnectionTest.java: 'App exited unexpectedly with 2'

2018-10-10 Thread Alex Menkov
Hi all, please review a fix for https://bugs.openjdk.java.net/browse/JDK-8195703 webrev: http://cr.openjdk.java.net/~amenkov/BasicJDWPConn/webrev.01/ I was not able to reproduce the issue, but accordingly the logs in jira root cause is a transport initialization error "Address already in use".

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-10 Thread David Holmes
On 11/10/2018 3:10 AM, Hohensee, Paul wrote: There used to be a rule against using “namespace”. Don’t know if that’s still true or not: I believe it is. Hence the “static const ”. Namespaces are used in a number of places now. There is still a prohibition on "using namespace ...". David You

Re: RFR (S) 8211432: [REDO] Handle JNIGlobalRefLocker.cpp

2018-10-10 Thread serguei.spit...@oracle.com
Hi Jc, +1 Thanks, Serguei On 10/10/18 12:10, Alex Menkov wrote: Looks good. --alex On 10/09/2018 16:09, JC Beyler wrote: Hi Paul, Yes base_msg can be doubly const so I did that in my local version. I'll push a new webrev once I get more reviews :) Thanks! Jc On Tue, Oct 9, 2018 at 3:5

Re: RFR (S) 8211432: [REDO] Handle JNIGlobalRefLocker.cpp

2018-10-10 Thread Alex Menkov
Looks good. --alex On 10/09/2018 16:09, JC Beyler wrote: Hi Paul, Yes base_msg can be doubly const so I did that in my local version. I'll push a new webrev once I get more reviews :) Thanks! Jc On Tue, Oct 9, 2018 at 3:54 PM Hohensee, Paul > wrote: Is the

Re: RFR (XL) 8211801: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[A-E]

2018-10-10 Thread JC Beyler
Thanks both for the reviews :) Jc On Wed, Oct 10, 2018 at 10:21 AM Alex Menkov wrote: > +1 > > --alex > > On 10/08/2018 15:31, serguei.spit...@oracle.com wrote: > > Hi Jc, > > > > Thank you for the update! > > It looks good to me. > > > > Thanks, > > Serguei > > > > > > On 10/8/18 14:53, JC Beyl

Re: RFR (XL) 8211801: Remove the NSK_CPP_STUB macros from vmTestbase for jvmti/scenarios/[A-E]

2018-10-10 Thread Alex Menkov
+1 --alex On 10/08/2018 15:31, serguei.spit...@oracle.com wrote: Hi Jc, Thank you for the update! It looks good to me. Thanks, Serguei On 10/8/18 14:53, JC Beyler wrote: Hi Serguei, Normally, I try not to fix issues that already exist before the change. For the space after the NULL for e

Re: RFR (S) 8211980: Remove ThreadHeapSampler enable/disable/enabled methods

2018-10-10 Thread Hohensee, Paul
There used to be a rule against using “namespace”. Don’t know if that’s still true or not: I believe it is. Hence the “static const ”. You only need OrderAccess if there could be a race on accesses to whatever you’re guarding. Looks like the old code wanted to make sure that log_table was initi