Hi Serguei,

Thanks for the review. I'll fix the missing space and run the java/lang/instrument tests.


thanks,

Chris

On 9/21/16 1:25 AM, serguei.spit...@oracle.com wrote:
Chris,

Forgot to tell about the testing.
You also need to run all the JTREG java/lang/instrument tests.

Thanks,
Serguei


On 9/21/16 01:21, serguei.spit...@oracle.com wrote:
Hi Chris,

It looks good.

One minor comment:

http://cr.openjdk.java.net/~cjplummer/8161225/webrev.00/webrev.hotspot/src/share/vm/prims/jvmtiEnter.xsl.frames.html

491 JvmtiUtil::error_name(JVMTI_ERROR_WRONG_PHASE),JvmtiEnv::get_phase());


  A space is missed after the comma.

Thanks,
Serguei



On 9/20/16 22:07, Chris Plummer wrote:
Hello,

Please help review the following:

https://bugs.openjdk.java.net/browse/JDK-8161225
http://cr.openjdk.java.net/~cjplummer/8161225/webrev.00/

The main fix is in JPLISAgent.c, which is to no longer call jplis_assert_msg() when there is a PHASE error, and also remove the test from ProblemList.txt.

I also fixed a problem with the test. It was not checking if the subprocess had failed to terminate properly. The result was if the sub-process crashed, then the test would pass. I noticed this when I temporarily forced an assert when there was a PHASE error, and suddenly the test was always passing, yet there was an hs_err.log file.

Lastly, I made a slight improvement to the trace output when there is a PHASE error, so now the PHASE number is included in the trace output. So the trace output now looks like the following when the test triggers the PHASE error (this is without the fix being made to the test):

[0.376s][trace][jvmti] [-] GetNamedModule JVMTI_ERROR_WRONG_PHASE(8)

Tested by running the test 5 times on each supported platform, and also ran nsk.jvmti and jck/vm/jvmti.

thanks,

Chris



Reply via email to