PING: RFR: 8165736: Error message should be shown when JVMTI agent cannot be attached
David Holmes
david.holmes at oracle.com
Tue Nov 28 21:17:23 UTC 2017
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 resolved your concern about
> Agent_OnAttach and testcase fix in StartManagementAgent.java.
Yes - Reviewed.
Thanks,
David
>
> Yasumasa
>
>
> 2017-11-28 16:20 GMT+09:00 serguei.spitsyn at oracle.com
> <serguei.spitsyn at 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 expected NumberFormatException.
>>>
>>> Thanks David!
>>> I updated it in new webrev:
>>>
>>> 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
>>
>>
>>
>>> I don't know about CSR well, so I want you to help about it if we need it.
>>>
>>> I think we need to merge this change to jdk repo ASAP because jdk10
>>> repo will be opened soon.
>>> I will change fixVersion to 11 if it is difficult.
>>>
>>> Of course, I want to merge it to jdk10. :-)
>>>
>>>
>>> Yasumasa
>>>
>>>
>>> 2017-11-28 15:21 GMT+09:00 David Holmes <david.holmes at oracle.com>:
>>>>
>>>> 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 causing problems for later
>>>>>> code.
>>>>>
>>>>>
>>>>> I removed the change to print pending exceptions in new webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.04/
>>>>>
>>>>>
>>>>>> test/jdk/com/sun/tools/attach/StartManagementAgent.java
>>>>>>
>>>>>> The reporting of NumberFormatException may be accurate in terms of the
>>>>>> low-level exception, but "Invalid com.sun.management.jmxremote.port
>>>>>> number"
>>>>>> was much clearer. This makes me wonder about whether the code that
>>>>>> previously produced "Invalid com.sun.management.jmxremote.port number"
>>>>>> needs
>>>>>> updating if this change proceeds. (And alao makes me wonder about the
>>>>>> impact
>>>>>> of the change in general.)
>>>>>
>>>>>
>>>>> I tested StartManagementAgent.java without this change, and I got
>>>>> failure
>>>>> as below:
>>>>> --------------
>>>>> JavaTest Message: Test threw exception:
>>>>> com.sun.tools.attach.AttachOperationFailedE
>>>>> xception: java.lang.RuntimeException:
>>>>> jdk.internal.agent.AgentConfigurationError: j
>>>>> ava.lang.NumberFormatException: For input string: "apa"
>>>>> JavaTest Message: shutting down test
>>>>>
>>>>> STATUS:Failed.`main' threw exception:
>>>>> com.sun.tools.attach.AttachOperationFailedExc
>>>>> eption: java.lang.RuntimeException:
>>>>> jdk.internal.agent.AgentConfigurationError: jav
>>>>> a.lang.NumberFormatException: For input string: "apa"
>>>>> --------------
>>>>>
>>>>> Should we change this testcase whatever this change is not accepted?
>>>>
>>>>
>>>> Obviously the test was not updated when the exception information
>>>> changed.
>>>> The code that generates the AgentConfigurationError is here:
>>>>
>>>> public static synchronized JMXConnectorServer
>>>> startRemoteConnectorServer(String portStr, Properties props) {
>>>>
>>>> // Get port number
>>>> final int port;
>>>> try {
>>>> port = Integer.parseInt(portStr);
>>>> } catch (NumberFormatException x) {
>>>> throw new AgentConfigurationError(INVALID_JMXREMOTE_PORT, x,
>>>> portStr);
>>>> }
>>>>
>>>> though I still can't see exactly how the printed exception information
>>>> would
>>>> come about. It makes me think that the code that sends the ACE back to
>>>> the
>>>> originating VM was updated inappropriately ... which may mean it was one
>>>> of
>>>> the earlier fixes in this area that broke the test.
>>>>
>>>> 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
>>>>
>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>> On 2017/11/20 6:41, David Holmes wrote:
>>>>>>
>>>>>> 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: printing of the pending exception.
>>>>>> If we
>>>>>> print the pending exception we will report an error and throw
>>>>>> AgentLoadException, even if execution of the OnAttach function returned
>>>>>> JNI_OK. If that exception was not critical to the success of the
>>>>>> loading the
>>>>>> agent, and the agent was just sloppy about clearing it, then it will
>>>>>> now
>>>>>> fail to load - which would be a compatibility concern.
>>>>>>
>>>>>> Further, if the exception indicates an error and the OnAttach function
>>>>>> returns JNI_ERR then we won't report that cleanly because the printing
>>>>>> of
>>>>>> the exception will prevent matching with "return code: -1".
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>> Some specific comments:
>>>>>>
>>>>>> HotSpotVirtualMachine.java
>>>>>>
>>>>>> The regex code seems overkill for the basic parsing you are doing. You
>>>>>> just need to see if the strings starts with "return code: " and then
>>>>>> parse
>>>>>> the next bit as an integer to get the return code.
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> test/jdk/com/sun/tools/attach/StartManagementAgent.java
>>>>>>
>>>>>> The reporting of NumberFormatException may be accurate in terms of the
>>>>>> low-level exception, but "Invalid com.sun.management.jmxremote.port
>>>>>> number"
>>>>>> was much clearer. This makes me wonder about whether the code that
>>>>>> previously produced "Invalid com.sun.management.jmxremote.port number"
>>>>>> needs
>>>>>> updating if this change proceeds. (And alao makes me wonder about the
>>>>>> impact
>>>>>> of the change in general.)
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Sorry - not the quick second review you were looking for.
>>>>>>
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>> On 19/11/2017 11:38 PM, Yasumasa Suenaga wrote:
>>>>>>>
>>>>>>> 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.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.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.03/
>>>>>>>>
>>>>>>>>
>>>>>>>> I've tested it as below. It works fine:
>>>>>>>>
>>>>>>>> $ $JT_HOME/bin/jtreg -ignore:quiet -nativepath:$NATIVE_PATH
>>>>>>>> hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
>>>>>>>> $ echo $NATIVE_PATH
>>>>>>>> /<Path to configuration>/images/test/hotspot/jtreg/native
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2017/11/16 16:49, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>
>>>>>>>>> On 11/15/17 23:29, David Holmes wrote:
>>>>>>>>>>
>>>>>>>>>> On 16/11/2017 4:43 PM, serguei.spitsyn at 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 should not be.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The test logic is adding it in AttachFailedTestBase.java:
>>>>>>>>>>
>>>>>>>>>> 45 return
>>>>>>>>>> Paths.get(System.getProperty("test.nativepath"),
>>>>>>>>>> "lib", libname)
>>>>>>>>>> 46 .toAbsolutePath()
>>>>>>>>>> 47 .toString();
>>>>>>>>>>
>>>>>>>>>> but it shouldn't.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Nice catch!
>>>>>>>>> I looked right to these lines and overlooked it. :)
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Serguei
>>>>>>>>>
>>>>>>>>>> David
>>>>>>>>>> -----
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> This is the script I'm using to run the tests:
>>>>>>>>>>>
>>>>>>>>>>> #!/bin/sh
>>>>>>>>>>>
>>>>>>>>>>> REPO=/var/tmp/sspitsyn/jdk.attach
>>>>>>>>>>> IMAGES=${REPO}/build/linux-x86_64-normal-server-release/images
>>>>>>>>>>> export JAVA_HOME=${IMAGES}/jdk
>>>>>>>>>>> export NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native
>>>>>>>>>>> export NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native
>>>>>>>>>>> echo "JAVA_HOME = $JAVA_HOME"
>>>>>>>>>>>
>>>>>>>>>>> /java/re/jtreg/4.2/nightly/binaries/jtreg/bin/jtreg
>>>>>>>>>>> -nativepath:${NATIVE_PATH} \
>>>>>>>>>>>
>>>>>>>>>>> $REPO/open/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> This is a part of log with the reported error from the
>>>>>>>>>>> AttachException.jtr:
>>>>>>>>>>>
>>>>>>>>>>> [TestNG] Running:
>>>>>>>>>>> serviceability/dcmd/jvmti/AttachFailed/AttachException.java
>>>>>>>>>>>
>>>>>>>>>>> Running DCMD 'JVMTI.agent_load
>>>>>>>>>>>
>>>>>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>>>>>>>>>> through 'PidJcmdExecutor'
>>>>>>>>>>> Executing command
>>>>>>>>>>>
>>>>>>>>>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>>>>>>>>> 8689, JVMTI.agent_load
>>>>>>>>>>>
>>>>>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so]'
>>>>>>>>>>> Command returned with exit code 0
>>>>>>>>>>> ---------------- stdout ----------------
>>>>>>>>>>> 8689:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so
>>>>>>>>>>> was not loaded.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native*/lib*/libException.so:
>>>>>>>>>>> cannot open shared object file: No such file or directory
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> These are the locations of the libException.so:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/libException.so
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> build/linux-x86_64-normal-server-release/support/test/hotspot/jtreg/native/lib/libException.so
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> The tests fail with the
>>>>>>>>>>> "NATIVE_PATH=${IMAGES}/test/hotspot/jtreg/native"
>>>>>>>>>>> but pass with the "export
>>>>>>>>>>> NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native".
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> When the "export
>>>>>>>>>>> NATIVE_PATH=${IMAGES}/../support/test/hotspot/jtreg/native" is
>>>>>>>>>>> used
>>>>>>>>>>> the log has this line:
>>>>>>>>>>>
>>>>>>>>>>> Running DCMD 'JVMTI.agent_load
>>>>>>>>>>>
>>>>>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/../support/test/hotspot/jtreg/native*/lib*/libException.so'
>>>>>>>>>>> through 'JMXExecutor'
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Apparently, the sub-directory name "/lib" is added to the path.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Serguei
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> David
>>>>>>>>>>>>
>>>>>>>>>>>> On 16/11/2017 4:34 AM, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> 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 test
>>>>>>>>>>>>>>> $ $JT_HOME/bin/jtreg -ignore:quiet
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -nativepath:<builddir>/<confdir>/support/test/hotspot/jtreg/native/lib
>>>>>>>>>>>>>>> hotspot/jtreg/serviceability/attach
>>>>>>>>>>>>>>> hotspot/jtreg/serviceability/dcmd/jvmti
>>>>>>>>>>>>>>> jdk/com/sun/tools/attach
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>> I missed to add the -nativepath flag, sorry.
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Please check that:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> make test-image
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> followed by jtreg
>>>>>>>>>>>>>> -nativepath:<build-dir>/images/test/hotspot/jtreg/native
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> also works.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> It fails with the error:
>>>>>>>>>>>>>
>>>>>>>>>>>>> 63 Running DCMD 'JVMTI.agent_load
>>>>>>>>>>>>>
>>>>>>>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so'
>>>>>>>>>>>>> through 'PidJcmdExecutor'
>>>>>>>>>>>>> 64 Executing command
>>>>>>>>>>>>>
>>>>>>>>>>>>> '[/var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>>>>>>>>>>> 28407, JVMTI.agent_load
>>>>>>>>>>>>>
>>>>>>>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg
>>>>>>>>>>>>> /native/lib/libException.so]'
>>>>>>>>>>>>> 65 Command returned with exit code 0
>>>>>>>>>>>>> 66 ---------------- stdout ----------------
>>>>>>>>>>>>> 67 28407:
>>>>>>>>>>>>> 68
>>>>>>>>>>>>>
>>>>>>>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so
>>>>>>>>>>>>> was not loaded.
>>>>>>>>>>>>> 69
>>>>>>>>>>>>>
>>>>>>>>>>>>> /var/tmp/sspitsyn/jdk.attach/build/linux-x86_64-normal-server-release/images/test/hotspot/jtreg/native/lib/libException.so:
>>>>>>>>>>>>> cannot open shared object file: No such file or directory
>>>>>>>>>>>>> 70
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> It seems, the '/lib' folder is added to the nativepath.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa, could you, double check it please?
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm using the jtreg:
>>>>>>>>>>>>> /java/re/jtreg/4.2/promoted/latest/binaries/jtreg/bin/jtreg
>>>>>>>>>>>>>
>>>>>>>>>>>>> which is:
>>>>>>>>>>>>>
>>>>>>>>>>>>> % ls -l /java/re/jtreg/4.2/promoted/latest
>>>>>>>>>>>>> lrwxrwxrwx 1 uucp 143 7 Nov 6 21:49
>>>>>>>>>>>>> /java/re/jtreg/4.2/promoted/latest -> fcs/b10/
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Good news is that the attach-related tests from closed
>>>>>>>>>>>>>>>> repository are passed.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 2017/11/15 16:38, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 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/jdk.attach/build/linux-x86_64-normal-server-release/images/jdk/bin/jcmd,
>>>>>>>>>>>>>>>> 21951, JVMTI.agent_load
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so]'
>>>>>>>>>>>>>>>> 111 Command returned with exit code 0
>>>>>>>>>>>>>>>> 112 ---------------- stdout ----------------
>>>>>>>>>>>>>>>> 113 21951:
>>>>>>>>>>>>>>>> 114
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so was
>>>>>>>>>>>>>>>> not loaded.
>>>>>>>>>>>>>>>> 115
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> /var/tmp/sspitsyn/tst/jdk.attach/JTwork/scratch/null/lib/libException.so:
>>>>>>>>>>>>>>>> cannot open shared object file: No such file or directory
>>>>>>>>>>>>>>>> 116
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Good news is that the attach-related tests from closed
>>>>>>>>>>>>>>>> repository are passed.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 11/14/17 16:40, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> 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 we remove this comparison?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I don't understand. Why do remove?
>>>>>>>>>>>>>>>>>>> Would it better to check for the exception name instead?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I've changed to check exception name
>>>>>>>>>>>>>>>>>> (NumberFormatException)
>>>>>>>>>>>>>>>>>> in
>>>>>>>>>>>>>>>>>> StartManagementAgent.java.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I will sponsor this fix and run these tests before the
>>>>>>>>>>>>>>>>>>> push.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>>>>> I'm waiting for second reviewer.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> 2017-11-08 11:55 GMT+09:00 serguei.spitsyn at oracle.com
>>>>>>>>>>>>>>>>>> <serguei.spitsyn at oracle.com>:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On 11/6/17 04:31, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Hi Serguei,
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On 2017/11/06 20:06, serguei.spitsyn at 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-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
>>>>>>>>>>>>>>>>>>>>>>> com.sun.management.jmxremote.port number")) {
>>>>>>>>>>>>>>>>>>>>>>> + if (!ex.getMessage().contains("For input string:
>>>>>>>>>>>>>>>>>>>>>>> \"apa\"")) {
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> What is the motivation for this change?
>>>>>>>>>>>>>>>>>>>>>>> It seems, the original comparison is better.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> "ex" is AttachOperationFailedException.
>>>>>>>>>>>>>>>>>>>>>> We can get the result as below when we run
>>>>>>>>>>>>>>>>>>>>>> StartManagementAgent:
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> -------------
>>>>>>>>>>>>>>>>>>>>>> [runApplication] Error: Invalid
>>>>>>>>>>>>>>>>>>>>>> com.sun.management.jmxremote.port
>>>>>>>>>>>>>>>>>>>>>> number: apa
>>>>>>>>>>>>>>>>>>>>>> [runApplication]
>>>>>>>>>>>>>>>>>>>>>> jdk.internal.agent.AgentConfigurationError:
>>>>>>>>>>>>>>>>>>>>>> java.lang.NumberFormatException: For input string:
>>>>>>>>>>>>>>>>>>>>>> "apa"
>>>>>>>>>>>>>>>>>>>>>> [runApplication] at
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startRemoteConnectorServer(ConnectorBootstrap.java:336)
>>>>>>>>>>>>>>>>>>>>>> -------------
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> I think we should check exception message in
>>>>>>>>>>>>>>>>>>>>>> AttachOperationFailedException.
>>>>>>>>>>>>>>>>>>>>>> In fact, jtreg fails at this point in my environment.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 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 better to check for the exception name instead?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> What tests did you run to make sure there are no
>>>>>>>>>>>>>>>>>>>>>>> regressions?
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> I've tested the following testcases:
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> - hotspot/jtreg/serviceability/attach
>>>>>>>>>>>>>>>>>>>>>> - hotspot/jtreg/serviceability/dcmd/jvmti
>>>>>>>>>>>>>>>>>>>>>> - jdk/com/sun/tools/attach
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> There are more tests related to dynamic attach in
>>>>>>>>>>>>>>>>>>>>> closed,
>>>>>>>>>>>>>>>>>>>>> nsk.aod.testlist and 30+ attach tests in
>>>>>>>>>>>>>>>>>>>>> nsk.jvmti.testlist.
>>>>>>>>>>>>>>>>>>>>> I'm not sure, if they are included into any of the Mach5
>>>>>>>>>>>>>>>>>>>>> testing levels.
>>>>>>>>>>>>>>>>>>>>> Will need to check.
>>>>>>>>>>>>>>>>>>>>> We have to make sure these tests are still passed.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> I cannot access JPRT and closed testcases because I'm not
>>>>>>>>>>>>>>>>>>>> an Oracle
>>>>>>>>>>>>>>>>>>>> employee.
>>>>>>>>>>>>>>>>>>>> Can you run them with this change?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Ok.
>>>>>>>>>>>>>>>>>>> I will sponsor this fix and run these tests before the
>>>>>>>>>>>>>>>>>>> push.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> It seems, another update and one more review is needed.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> On 2017/11/03 16:31, serguei.spitsyn at oracle.com wrote:
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> 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")) {
>>>>>>>>>>>>>>>>>>>>>>> + if (!ex.getMessage().contains("For input string:
>>>>>>>>>>>>>>>>>>>>>>> \"apa\"")) {
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> What is the motivation for this change?
>>>>>>>>>>>>>>>>>>>>>>> It seems, the original comparison is better.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachException.java.html
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachIncorrectLibrary.java.html
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachNoEntry.java.html
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/test/hotspot/jtreg/serviceability/dcmd/jvmti/AttachFailed/AttachReturnError.java.html
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> 37 public void run(CommandExecutor executor)
>>>>>>>>>>>>>>>>>>>>>>> {
>>>>>>>>>>>>>>>>>>>>>>> 38 try{
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> A space is missed after 'try'.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> It is odd that all test java classes define
>>>>>>>>>>>>>>>>>>>>>>> exactly
>>>>>>>>>>>>>>>>>>>>>>> the same
>>>>>>>>>>>>>>>>>>>>>>> methods: sharedObjectName(), jmx() and cli().
>>>>>>>>>>>>>>>>>>>>>>> Would it better to defin a common base class with
>>>>>>>>>>>>>>>>>>>>>>> these methods?
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Otherwise, it looks good.
>>>>>>>>>>>>>>>>>>>>>>> Thank you for taking care about it!
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> What tests did you run to make sure there are no
>>>>>>>>>>>>>>>>>>>>>>> regressions?
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>> Serguei
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> On 11/1/17 05:59, Yasumasa Suenaga wrote:
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> 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". However,
>>>>>>>>>>>>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>>>>>>>>>>> implies error
>>>>>>>>>>>>>>>>>>>>>>>>> in
>>>>>>>>>>>>>>>>>>>>>>>>> JVMTIAgentLoadDCmd.
>>>>>>>>>>>>>>>>>>>>>>>>> This message is from JCmd.java when jcmd does not
>>>>>>>>>>>>>>>>>>>>>>>>> receive any output
>>>>>>>>>>>>>>>>>>>>>>>>> from target VM.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> I think HotSopt/jcmd should return useful error
>>>>>>>>>>>>>>>>>>>>>>>>> message to users to
>>>>>>>>>>>>>>>>>>>>>>>>> understand the cause of failure.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> I uploaded webrev for this issue. Could you review
>>>>>>>>>>>>>>>>>>>>>>>>> it?
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8165736/webrev.00/
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> This change is work fine on Fedora 26 x86_64 as
>>>>>>>>>>>>>>>>>>>>>>>>> following jtreg
>>>>>>>>>>>>>>>>>>>>>>>>> testcases:
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> - hotspot/jtreg/serviceability/attach
>>>>>>>>>>>>>>>>>>>>>>>>> - hotspot/jtreg/serviceability/dcmd/jvmti
>>>>>>>>>>>>>>>>>>>>>>>>> - jdk/com/sun/tools/attach
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>>>>>>>>>>>>>>>>>>>>> (I cannot test it on other platforms.)
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>>>>>>>>>>>>>
>>
More information about the serviceability-dev
mailing list