Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-28 Thread David Holmes
On 28/11/2017 5:35 PM, Yasumasa Suenaga wrote: http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.06/ The update looks good to me. Serguei, do we need CSR? After some thinking, I'd say - Not. Thanks Serguei! David, can I list you as a reviewer? I guess that latest webrev is re

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-27 Thread Yasumasa Suenaga
>>http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.06/ > > > The update looks good to me. > >> Serguei, do we need CSR? > > > After some thinking, I'd say - Not. Thanks Serguei! David, can I list you as a reviewer? I guess that latest webrev is resolved your concern about Agent_OnAttac

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-27 Thread serguei.spit...@oracle.com
Hi Yasumasa, On 11/27/17 23:15, Yasumasa Suenaga wrote: Hi, That said perhaps your simple fix to the test suffices here as the code is simply trying to skip throwing the expected exception. Perhaps include the whole of 'NumberFormatException: For input string: "apa"' just to be sure it is the

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-27 Thread Yasumasa Suenaga
Hi, > That said perhaps your simple fix to the test suffices here as the code is > simply trying to skip throwing the expected exception. Perhaps include the > whole of 'NumberFormatException: For input string: "apa"' just to be sure it > is the expected NumberFormatException. Thanks David! I upd

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-27 Thread David Holmes
Hi Yasumasa, No further comments from me. Thanks, David On 22/11/2017 12:48 AM, Yasumasa Suenaga wrote: Hi Serguei, On 2017/11/21 16:17, serguei.spit...@oracle.com wrote: Hi Yasumasa, Thank you for the update. Some comments. http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.04/src/j

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-27 Thread David Holmes
Sorry Yasumasa I was away for a few days. On 20/11/2017 5:54 PM, Yasumasa Suenaga wrote: Hi David, My own feeling is that it is up to the OnAttach function to properly deal with pending exceptions: report and/or clear them. The VM side just has to clear any pending exception to avoid it caus

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-21 Thread Yasumasa Suenaga
Hi Serguei, On 2017/11/21 16:17, serguei.spit...@oracle.com wrote: Hi Yasumasa, Thank you for the update. Some comments. http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.04/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java.frames.html 94 BufferedReader reader =

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-20 Thread serguei.spit...@oracle.com
Hi Yasumasa, Thank you for the update. Some comments. http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.04/src/jdk.attach/share/classes/sun/tools/attach/HotSpotVirtualMachine.java.frames.html 94 BufferedReader reader = new BufferedRe

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-19 Thread Yasumasa Suenaga
Hi David, My own feeling is that it is up to the OnAttach function to properly deal with pending exceptions: report and/or clear them. The VM side just has to clear any pending exception to avoid it causing problems for later code. I removed the change to print pending exceptions in new web

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-19 Thread David Holmes
Hi Yasumasa, I've been trying to leave these reviews to serviceability folk ... I've gone back through the original RFR from September last year to see what we did and what was left. The current proposal raises some concern for me - and IIRC Dmitry was also concerned about it last time: prin

PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-19 Thread Yasumasa Suenaga
PING: Could you review it? http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.03/ I want to merge this change to jdk 10. So I need a second reviewer. Yasumasa On 2017/11/16 21:09, Yasumasa Suenaga wrote: Hi David, Serguei, The test logic is adding it in AttachFailedTestBase.ja

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-16 Thread Yasumasa Suenaga
Hi David, Serguei, The test logic is adding it in AttachFailedTestBase.java: 45 return Paths.get(System.getProperty("test.nativepath"), "lib", libname) 46 .toAbsolutePath() 47 .toString(); Thanks! I've fixed it in new webrev: http://cr

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-15 Thread serguei.spit...@oracle.com
On 11/15/17 23:29, David Holmes wrote: On 16/11/2017 4:43 PM, serguei.spit...@oracle.com wrote: On 11/15/17 18:11, David Holmes wrote: Hi Serguei, > /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' There should not

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-15 Thread David Holmes
On 16/11/2017 4:43 PM, serguei.spit...@oracle.com wrote: On 11/15/17 18:11, David Holmes wrote: Hi Serguei, > /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' There should not be any "/lib/" in that path Right, it

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-15 Thread serguei.spit...@oracle.com
On 11/15/17 18:11, David Holmes wrote: Hi Serguei, > /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' There should not be any "/lib/" in that path

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-15 Thread David Holmes
Hi Serguei, > /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so' There should not be any "/lib/" in that path David On 16/11/2017 4:34 AM, serguei.spit...@oracle.com wrote: Hi Yasumasa and David, On 11/15/17 04:56,

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-15 Thread serguei.spit...@oracle.com
Hi Yasumasa and David, On 11/15/17 04:56, David Holmes wrote: On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote: Hi Serguei, Do the new tests pass in your runs? Of course. It seems not to exist jtreg native libraries. I've tested as below:    $ make build-test-hotspot-jtreg-native    $ cd te

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-15 Thread David Holmes
On 15/11/2017 10:15 PM, Yasumasa Suenaga wrote: Hi Serguei, Do the new tests pass in your runs? Of course. It seems not to exist jtreg native libraries. I've tested as below:   $ make build-test-hotspot-jtreg-native   $ cd test   $ $JT_HOME/bin/jtreg -ignore:quiet -nativepath://support/t

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-15 Thread Yasumasa Suenaga
Hi Serguei, Do the new tests pass in your runs? Of course. It seems not to exist jtreg native libraries. I've tested as below: $ make build-test-hotspot-jtreg-native $ cd test $ $JT_HOME/bin/jtreg -ignore:quiet -nativepath://support/test/hotspot/jtreg/native/lib hotspot/jtreg/servicea

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-14 Thread serguei.spit...@oracle.com
Hi Yasumasa, Do the new tests pass in your runs? In my runs 3 of 4 tests are failed with the errors like this:  109 Running DCMD 'JVMTI.agent_load /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so' through 'PidJcmdExecutor'  110 Executing command '[/var/tmp/sspitsyn/jd

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-14 Thread serguei.spit...@oracle.com
Hi Yasumasa, It looks good to me. Thanks, Serguei On 11/7/17 22:38, Yasumasa Suenaga wrote: Hi Serguei, I uploaded new webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/ I'd expect a check for some exception name, not for details like: For input string: "apa". Should

PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-13 Thread Yasumasa Suenaga
PING: Could you reivew it? We need one more reviewer. > http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/ Thanks, Yasumasa 2017-11-08 15:38 GMT+09:00 Yasumasa Suenaga : > Hi Serguei, > > I uploaded new webrev: > > http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/ > >>>

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-07 Thread Yasumasa Suenaga
Hi Serguei, I uploaded new webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.02/ >>> I'd expect a check for some exception name, not for details like: For >>> input string: "apa". >> >> >> Should we remove this comparison? > > > I don't understand. Why do remove? > Would it bette

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-07 Thread serguei.spit...@oracle.com
On 11/6/17 04:31, Yasumasa Suenaga wrote: Hi Serguei, On 2017/11/06 20:06, serguei.spit...@oracle.com wrote: Hi Yasumasa, The changes looks good. Thank you for making them! Thanks! On 11/3/17 05:10, Yasumasa Suenaga wrote: Hi Serguei, Thank you for your comment! I uploaded new webrev:

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-06 Thread Yasumasa Suenaga
Hi Serguei, On 2017/11/06 20:06, serguei.spit...@oracle.com wrote: Hi Yasumasa, The changes looks good. Thank you for making them! Thanks! On 11/3/17 05:10, Yasumasa Suenaga wrote: Hi Serguei, Thank you for your comment! I uploaded new webrev:   http://cr.openjdk.java.net/~ysuenaga/JDK-

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-06 Thread serguei.spit...@oracle.com
Hi Yasumasa, The changes looks good. Thank you for making them! On 11/3/17 05:10, Yasumasa Suenaga wrote: Hi Serguei, Thank you for your comment! I uploaded new webrev:   http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.01/ http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-03 Thread Yasumasa Suenaga
Hi Serguei, Thank you for your comment! I uploaded new webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.01/ http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/jdk/com/sun/tools/attach/StartManagementAgent.java.udiff.html - if (!ex.getMessage().contains("Invalid

Re: PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-03 Thread serguei.spit...@oracle.com
Hi Yasumasa, Some comments. http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/jdk/com/sun/tools/attach/StartManagementAgent.java.udiff.html -if (!ex.getMessage().contains("Invalid com.sun.management.jmxremote.port number")) { +

PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached

2017-11-01 Thread Yasumasa Suenaga
PING: Could you review and sponsor it? http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/ Thanks, Yasumasa On 2017/09/29 13:24, Yasumasa Suenaga wrote: Hi all, If we try to attach invalid JVMTI agent via JVMTI.agent_load dcmd, we will get "Command executed successfully". Howe