RFR(S) 8161225: Assert failure in JVMTI GetNamedModule at JPLISAgent.c line: 792
chris.plummer at oracle.com
Wed Sep 21 16:33:55 UTC 2016
On 9/21/16 7:33 AM, Daniel D. Daugherty wrote:
> On 9/20/16 11:07 PM, Chris Plummer wrote:
>> Please help review the following:
> L490: log_trace(jvmti)("[-] %s %s(%d)", func_name,
> nit: extra space after the comma
> nit: extra space at the end of the line
Those were both pre-existing, but I'll clean them up.
> No comments.
> No comments.
> I presume when you change the assertTrue() call to an
> OutputAnalyzer.should...() call that the new import can
> be deleted.
Correct. I have removed it and it still compiles.
> One thing that wasn't clear from your write up below... when you detect
> a PHASE error, you now return a NULL (but the code is clear).
> Thumbs up. Don't need to see a new webrev for fixing any nits.
Thanks for the review.
>> 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.
More information about the hotspot-runtime-dev