<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hello.<br>
<br>
I have a customer with non-escalated bug that is duplicate of this
bug. I checked with patch from webrev.03 and can confirm that fixed
the bug.<br>
<br>
Could you guys approve it if it's fine?<br>
<div class="moz-cite-prefix"><br>
Thanks. Vladimir.<br>
On 24.04.2013 22:35, Tao Mao wrote:<br>
</div>
<blockquote cite="mid:517825D4.4020505@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
For 32-bit builds: The current changeset provides the first
"protection" of heap size handling. Then comes handling whether we
can allocate a certain amount of a heap.<br>
<br>
For 64-bit builds: a machine with 512M-1GB memory should fail in
many other tests as well, due to inability to lauch jvm.<br>
<br>
Anyway, I ran the test and it passed. <br>
<br>
--------------------------------------------------<br>
Simply,<br>
<br>
-bash-4.1$ ulimit -v 134217728 (128m for human reading)<br>
-bash-4.1$ ulimit -m 134217728 (128m FHR)<br>
<br>
(cannot limit virtual memory to zero or set vm.swappiness=0, in
which case the server would hang immediately w/o even launching
jvm. But, the setting should satisfy your question.)<br>
<br>
Then, run jtreg. Then, pass.<br>
--------------------------------------------------<br>
<br>
BTW, since you've jumped in this thread ^_^ can you check the way
I get system property through "test.java.opts"
<meta charset="utf-8">
to set inside jvm process? It's in
test/gc/init/TestHandleExceedingProcessSizeLimitIn32BitBuilds.java<br>
<br>
Thank you.<br>
Tao<br>
<br>
On 4/24/13 3:36 AM, Leonid Mesnik wrote:
<blockquote cite="mid:5177B599.3050604@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 04/22/2013 10:07 PM, Tao Mao
wrote:<br>
</div>
<blockquote cite="mid:51757C74.9000605@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
This should be handled by a separate CR (probably by this CR <a
moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://jbs.oracle.com/bugs/browse/JDK-7112912">https://jbs.oracle.com/bugs/browse/JDK-7112912</a>)<br>
<br>
</blockquote>
Why this? I mean should your test pass on hosts with *small*
amount of memory. <br>
<br>
Leonid<br>
<blockquote cite="mid:51757C74.9000605@oracle.com" type="cite">
Thanks.<br>
Tao<br>
<br>
On 4/22/13 2:46 AM, Leonid Mesnik wrote:
<blockquote cite="mid:517506EE.5030000@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Tao <br>
<br>
This is not review, just question. <br>
<br>
Should you test pass on hosts with 512M-1GB memory and no
swap? <br>
<br>
Leonid<br>
On 04/19/2013 09:27 PM, Tao Mao wrote:<br>
</div>
<blockquote cite="mid:51717E6E.4020806@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
1. check overflow routine is wrapped up.<br>
<br>
2. The reduction of code duplication is adopted.<br>
<br>
3. Develop a way to intake 0~multiple external vm options
in jtreg main(). Hope it helps others implement similar
functionality.<br>
<br>
webrev:<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/6761744/webrev.02/">http://cr.openjdk.java.net/~tamao/6761744/webrev.02/</a><br>
<br>
test:<br>
JTREG: passed.<br>
jtreg -jdk:/home/tamao/jdk1.7.0_04-i586 -vmoptions:"-tamao
<GC_OPTION>"
/home/tamao/src/6761744CrashIfProcessSizeLimitExceeded_hsx24/test/gc/init/TestHandleExceedingProcessSizeLimitIn32BitBuilds.java<br>
<br>
where GC_OPTION rotates in -XX:+UseParallelGC -XX:+UseG1GC
-XX:+UseSerialGC -XX:+UseParNewGC -XX:+UseConcMarkSweepGC<br>
<br>
<br>
Thanks.<br>
Tao<br>
<br>
On 4/15/13 1:37 AM, Bengt Rutisson wrote:
<blockquote cite="mid:516BBC64.6060509@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
Hi Tao,<br>
<br>
I agree with John. The changes look fine and I'm ok
with them. But it would look nicer to add the
CollectedHeap::add_and_verify_no_overflow() method.<br>
<br>
Also, regarding the test, a couple of minor things.<br>
<br>
First, with the new naming convention I think the test
should be called something like:<br>
<br>
test/gc/init/TestHandleExceedingProcessSizeLimitOn32BitSystems.java<br>
<br>
Then I think you can reduce the code duplication a
bit. You create the same ProcessBuilder and
OutputAnalyzer in both 32 and 64 bit cases. So maybe
it could look like this instead:<br>
<br>
<tt>public class
TestHandleExceedingProcessSizeLimitOn32BitSystems {</tt><tt><br>
</tt><tt> public static void main(String args[])
throws Exception {</tt><tt><br>
</tt><tt> ProcessBuilder pb =</tt><tt><br>
</tt><tt>
ProcessTools.createJavaProcessBuilder(System.getProperty("test.vm.opts"),</tt><tt><br>
</tt><tt>
"-Xmx3072m",</tt><tt><br>
</tt><tt>
"-XX:MaxPermSize=1024m",</tt><tt><br>
</tt><tt>
"-version");</tt><tt><br>
</tt><tt> OutputAnalyzer output = new
OutputAnalyzer(pb.start());</tt><tt><br>
</tt><tt><br>
</tt><tt> String dataModel =
System.getProperty("sun.arch.data.model");</tt><tt><br>
</tt><tt> if (dataModel.equals("32")) {</tt><tt><br>
</tt><tt> output.shouldContain("The size of the
object heap + perm gen exceeds the maximum
representable size");</tt><tt><br>
</tt><tt> if (output.getExitValue() == 0) {</tt><tt><br>
</tt><tt> throw new RuntimeException("Not
expected to get exit value 0");</tt><tt><br>
</tt><tt> }</tt><tt><br>
</tt><tt> } else if (dataModel.equals("64")) {</tt><tt><br>
</tt><tt> output.shouldHaveExitValue(0);</tt><tt><br>
</tt><tt> }</tt><tt><br>
</tt><tt> }</tt><tt><br>
</tt><tt>}</tt><tt><br>
</tt><br>
There is also a small bug in the test. If you run
JTreg without passing any vmoptions the test will
fail. The reason is that
System.getProperty("test.vm.opts") will evaluate to
the empty string. This will be passed as the first
argument to the Java process, which will assume that
this is the class name. So, it fails to start because
it can't load the class <empty string>.<br>
<br>
To reproduce the problem use this command line:<br>
<br>
java -jar <path_to_jtreg>/lib/jtreg.jar
test/gc/6761744/TestHandleExceedingProcessSizeLimitOn32BitSystems.java<br>
<br>
I guess the fix is to check that
System.getProperty("test.vm.opts") is not empty before
passing it on to createJavaProcessBuilder().<br>
<br>
Thanks,<br>
Bengt<br>
<br>
On 4/13/13 3:15 AM, John Cuthbertson wrote:<br>
</div>
<blockquote cite="mid:5168B1AD.7060302@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
Hi Tao,<br>
<br>
This looks OK to me.<br>
<br>
I can see what Thomas is saying though. All of the
checks involve something like:<br>
<br>
total += some_value;<br>
if (total < some_value) {<br>
// We must have overflowed<br>
vm_exit(...);<br>
}<br>
<br>
So a function in CollectedHeap (the base class of
SharedHeap and ParallelScavengeHeap) might make sense.
For example:<br>
<br>
total_reserved = 0;<br>
total_reserved =
add_and_verify_no_overflow(total_reserved,
max_heap_size);<br>
total_reserved =
add_and_verify_no_overflow(total_reserved,
max_perm_size);<br>
<br>
Where the function is:<br>
<br>
size_t add_and_verify_no_overflow(size_t x, size_t y)
{<br>
const char* msg = "...";<br>
x += y;<br>
if (x < y) {<br>
vm_exit(msg);<br>
}<br>
return x;<br>
}<br>
<br>
But I can live with your current changes.<br>
<br>
JohnC<br>
<br>
<div class="moz-cite-prefix">On 4/10/2013 9:52 AM, Tao
Mao wrote:<br>
</div>
<blockquote cite="mid:516598D3.1020809@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
Hi Bengt,<br>
<br>
Thank you for reviewing. A new webrev is updated.<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/6761744/webrev.01/">http://cr.openjdk.java.net/~tamao/6761744/webrev.01/</a><br>
<br>
Cheers,<br>
Tao<br>
<br>
On 4/10/13 1:54 AM, Bengt Rutisson wrote:
<blockquote cite="mid:516528C2.8080709@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
Hi Tao,<br>
<br>
This change looks good. Thanks for adding the
JTReg test, it looks good!<br>
<br>
One minor nit:<br>
<br>
In
src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
I would suggest to
<meta http-equiv="content-type"
content="text/html; charset=UTF-8">
store "(size_t)
align_size_up(pgs->max_size(),
HeapRegion::GrainBytes)" in a local variable
rather than duplicating the calculation.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
On 4/8/13 7:22 AM, Tao Mao wrote:<br>
</div>
<blockquote cite="mid:51625424.4050209@oracle.com"
type="cite">
<meta http-equiv="content-type"
content="text/html; charset=UTF-8">
6761744 Hotspot crashes if process size limit is
exceeded<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://jbs.oracle.com/bugs/browse/JDK-6761744">https://jbs.oracle.com/bugs/browse/JDK-6761744</a><br>
<br>
webrev:<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Etamao/6761744/webrev.00/">http://cr.openjdk.java.net/~tamao/6761744/webrev.00/</a><br>
<br>
changeset:<br>
The fix only needs to go to hsx24 since there's
no perm gen in hotspot-25. Thus, the webrev is
based on hsx24 repo.<br>
<br>
It provides for 32-bit builds a preventive check
of the size of "the object heap + perm gen"
before reserving VM memory. The total size
should not exceed 4096MB (i.e. 4GB) for 32-bit
builds; otherwise, the total doesn't make sense
and, what's worse, overflow occurs. It will
consequentially trigger anther error of memory
access violation, which was not protected<span
style="color: rgb(0, 0, 0); font-family:
Arial, FreeSans, Helvetica, sans-serif;
font-size: 12.727272033691406px; font-style:
normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height:
15.454545021057129px; orphans: auto;
text-align: start; text-indent: 0px;
text-transform: none; white-space: normal;
widows: auto; word-spacing: 0px;
-webkit-text-size-adjust: auto;
-webkit-text-stroke-width: 0px;
background-color: rgb(240, 240, 240); display:
inline !important; float: none;"></span>.<br>
<br>
jtreg testing java code is also written,
checking both 32-bit and (trivially) 64-bit
builds.<br>
<br>
testing:<br>
check jtreg tests with flags -XX:+UseParallelGC,
-XX:+UseG1GC, -XX:+UseParNewGC,
-XX:+UseConcMarkSweepGC, -XX:+UseSerialGC and
builds of 32-bit and 64-bit. All passed.<br>
<br>
Needs JPRT test when pushing.<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Leonid Mesnik
JVM SQE</pre>
</blockquote>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Leonid Mesnik
JVM SQE</pre>
</blockquote>
</blockquote>
<br>
</body>
</html>