Please review: 8066185: VM crashed with SIGSEGV VirtualMemoryTracker::add_reserved_region

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Tue Feb 24 16:17:29 UTC 2015


Hi David,

comments inlined.

> Hi Kumar,
>
> On 24/02/2015 8:14 AM, Kumar Srinivasan wrote:
>> Hello,
>>
>> Please review the fix for the above issue.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~ksrini/8066185/webrev.00/
>>
>> The fix is self explanatory, as for the test I have done the following:
>
> I found the comment:
>
>           /*
> +          * Since this is a VM flag, we need to ensure it is not an
> +          * application argument, meaning the argument must precede
> +          * the main class or those flags that invoke the VM directly.
> +          */
>
> a bit confusing - specifically the "or those flags that invoke the VM 
> directly". Took me while to realize that all the args you treat 
> specially (-version, -h, -jar etc) are all "terminal" arguments - 
> either the launcher stops looking at args after the given arg, or all 
> following args must be for the application, not the launcher or VM. I 
> would have expressed this as:
>
>   /*
>    * Since this must be a VM flag we stop processing once we see
>    * an argument the launcher would not have processed beyond (such
>    * as -version or -h), or an argument that indicates the following
>    * arguments are for the application (i.e. the main class name, or
>    * the -jar argument).
>    */
>
>
+1,  I have updated the webrev with your comments, also had an AHA moment,
I realized, had missed -X, which prints the XUsage, added this to the logic
and test.

>> a. refactored it from a single longish test to sub-tests for 
>> readability.
>> b. added new sub-test for NMT Argument Processing checks.
>
> Can you please update the @summary for that test. It seems the OSX 
> specific test was hijacked for NMT arg testing and the summary was 
> never updated to reflect that.

This test was "commandeered" :-) for other purposes, updated the summary to
describe what is being performed.

Webrev to last reviewed:
http://cr.openjdk.java.net/~ksrini/8066185/webrev.01/webrev.delta/index.html

Full webrev:
http://cr.openjdk.java.net/~ksrini/8066185/webrev.01/index.html

Thanks.
Kumar

>
> Thanks,
> David
>
>> Thanks
>> Kumar
>>




More information about the core-libs-dev mailing list