Request for review: 6761744 Hotspot crashes if process size limit is exceeded
Tao Mao
tao.mao at oracle.com
Wed May 1 17:36:35 UTC 2013
I tried your reproducer on 7u21 and it had no complaint.
jdk/1.7.0_21/bin/java -XX:+UseParallelGC -Xmx4092M -version
java version "1.7.0_21-ea"
Java(TM) SE Runtime Environment (build 1.7.0_21-ea-b01)
Java HotSpot(TM) Server VM (build 23.7-b01, mixed mode)
Note that 4092M < 4G.
What was the symptom you got? How large is the tested machine's memory?
Thanks.
Tao
On 5/1/13 2:30 AM, Vladimir Kempik wrote:
> ParallelScavange and 7u21.
>
> What was happening - gc, when allocating memory, was overwriting some
> of it's own code and then calling bunch of zeroes.
>
> So I think safe memory increase and overflow check, from the patch,
> fixed the problem.
>
> Vladimir
>
>
>
> 01.05.2013, в 1:52, Tao Mao <tao.mao at oracle.com
> <mailto:tao.mao at oracle.com>> написал(а):
>
>> IMHO, what the customer reported could be a similar but different bug.
>>
>> What version did they use, and what gc?
>>
>> Thanks.
>> Tao
>>
>> On 4/30/13 1:22 PM, Vladimir Kempik wrote:
>>> Hi
>>>
>>> It has easy reproducer.
>>>
>>> In solaris sparc, run 32-bit java
>>>
>>> java -Xmx4092M -version
>>>
>>> Java6/7 crashes. With patch from webrev.03 it works as it should,
>>> saying it has not enough memory .
>>> Vladimir
>>>
>>> 30.04.2013, в 22:11, Tao Mao <tao.mao at oracle.com
>>> <mailto:tao.mao at oracle.com>> написал(а):
>>>
>>>> Hi Vladimir,
>>>>
>>>> Can you point me to your bug you think it's associated with?
>>>>
>>>> Thanks.
>>>> Tao
>>>>
>>>> On 4/30/13 3:33 AM, Vladimir Kempik wrote:
>>>>> Hello.
>>>>>
>>>>> 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.
>>>>>
>>>>> Could you guys approve it if it's fine?
>>>>>
>>>>> Thanks. Vladimir.
>>>>> On 24.04.2013 22:35, Tao Mao wrote:
>>>>>> 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.
>>>>>>
>>>>>> For 64-bit builds: a machine with 512M-1GB memory should fail in
>>>>>> many other tests as well, due to inability to lauch jvm.
>>>>>>
>>>>>> Anyway, I ran the test and it passed.
>>>>>>
>>>>>> --------------------------------------------------
>>>>>> Simply,
>>>>>>
>>>>>> -bash-4.1$ ulimit -v 134217728 (128m for human reading)
>>>>>> -bash-4.1$ ulimit -m 134217728 (128m FHR)
>>>>>>
>>>>>> (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.)
>>>>>>
>>>>>> Then, run jtreg. Then, pass.
>>>>>> --------------------------------------------------
>>>>>>
>>>>>> BTW, since you've jumped in this thread ^_^ can you check the way
>>>>>> I get system property through "test.java.opts" to set inside jvm
>>>>>> process? It's in
>>>>>> test/gc/init/TestHandleExceedingProcessSizeLimitIn32BitBuilds.java
>>>>>>
>>>>>> Thank you.
>>>>>> Tao
>>>>>>
>>>>>> On 4/24/13 3:36 AM, Leonid Mesnik wrote:
>>>>>>> On 04/22/2013 10:07 PM, Tao Mao wrote:
>>>>>>>> This should be handled by a separate CR (probably by this CR
>>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-7112912)
>>>>>>>>
>>>>>>> Why this? I mean should your test pass on hosts with *small*
>>>>>>> amount of memory.
>>>>>>>
>>>>>>> Leonid
>>>>>>>> Thanks.
>>>>>>>> Tao
>>>>>>>>
>>>>>>>> On 4/22/13 2:46 AM, Leonid Mesnik wrote:
>>>>>>>>> Tao
>>>>>>>>>
>>>>>>>>> This is not review, just question.
>>>>>>>>>
>>>>>>>>> Should you test pass on hosts with 512M-1GB memory and no swap?
>>>>>>>>>
>>>>>>>>> Leonid
>>>>>>>>> On 04/19/2013 09:27 PM, Tao Mao wrote:
>>>>>>>>>> 1. check overflow routine is wrapped up.
>>>>>>>>>>
>>>>>>>>>> 2. The reduction of code duplication is adopted.
>>>>>>>>>>
>>>>>>>>>> 3. Develop a way to intake 0~multiple external vm options in
>>>>>>>>>> jtreg main(). Hope it helps others implement similar
>>>>>>>>>> functionality.
>>>>>>>>>>
>>>>>>>>>> webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~tamao/6761744/webrev.02/
>>>>>>>>>>
>>>>>>>>>> test:
>>>>>>>>>> JTREG: passed.
>>>>>>>>>> jtreg -jdk:/home/tamao/jdk1.7.0_04-i586 -vmoptions:"-tamao
>>>>>>>>>> <GC_OPTION>"
>>>>>>>>>> /home/tamao/src/6761744CrashIfProcessSizeLimitExceeded_hsx24/test/gc/init/TestHandleExceedingProcessSizeLimitIn32BitBuilds.java
>>>>>>>>>>
>>>>>>>>>> where GC_OPTION rotates in -XX:+UseParallelGC -XX:+UseG1GC
>>>>>>>>>> -XX:+UseSerialGC -XX:+UseParNewGC -XX:+UseConcMarkSweepGC
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks.
>>>>>>>>>> Tao
>>>>>>>>>>
>>>>>>>>>> On 4/15/13 1:37 AM, Bengt Rutisson wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Tao,
>>>>>>>>>>>
>>>>>>>>>>> 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.
>>>>>>>>>>>
>>>>>>>>>>> Also, regarding the test, a couple of minor things.
>>>>>>>>>>>
>>>>>>>>>>> First, with the new naming convention I think the test
>>>>>>>>>>> should be called something like:
>>>>>>>>>>>
>>>>>>>>>>> test/gc/init/TestHandleExceedingProcessSizeLimitOn32BitSystems.java
>>>>>>>>>>>
>>>>>>>>>>> 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:
>>>>>>>>>>>
>>>>>>>>>>> public class TestHandleExceedingProcessSizeLimitOn32BitSystems {
>>>>>>>>>>> public static void main(String args[]) throws Exception {
>>>>>>>>>>> ProcessBuilder pb =
>>>>>>>>>>>
>>>>>>>>>>> ProcessTools.createJavaProcessBuilder(System.getProperty("test.vm.opts"),
>>>>>>>>>>> "-Xmx3072m",
>>>>>>>>>>>
>>>>>>>>>>> "-XX:MaxPermSize=1024m",
>>>>>>>>>>> "-version");
>>>>>>>>>>> OutputAnalyzer output = new OutputAnalyzer(pb.start());
>>>>>>>>>>>
>>>>>>>>>>> String dataModel =
>>>>>>>>>>> System.getProperty("sun.arch.data.model");
>>>>>>>>>>> if (dataModel.equals("32")) {
>>>>>>>>>>> output.shouldContain("The size of the object heap +
>>>>>>>>>>> perm gen exceeds the maximum representable size");
>>>>>>>>>>> if (output.getExitValue() == 0) {
>>>>>>>>>>> throw new RuntimeException("Not expected to get exit
>>>>>>>>>>> value 0");
>>>>>>>>>>> }
>>>>>>>>>>> } else if (dataModel.equals("64")) {
>>>>>>>>>>> output.shouldHaveExitValue(0);
>>>>>>>>>>> }
>>>>>>>>>>> }
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> 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>.
>>>>>>>>>>>
>>>>>>>>>>> To reproduce the problem use this command line:
>>>>>>>>>>>
>>>>>>>>>>> java -jar <path_to_jtreg>/lib/jtreg.jar
>>>>>>>>>>> test/gc/6761744/TestHandleExceedingProcessSizeLimitOn32BitSystems.java
>>>>>>>>>>>
>>>>>>>>>>> I guess the fix is to check that
>>>>>>>>>>> System.getProperty("test.vm.opts") is not empty before
>>>>>>>>>>> passing it on to createJavaProcessBuilder().
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Bengt
>>>>>>>>>>>
>>>>>>>>>>> On 4/13/13 3:15 AM, John Cuthbertson wrote:
>>>>>>>>>>>> Hi Tao,
>>>>>>>>>>>>
>>>>>>>>>>>> This looks OK to me.
>>>>>>>>>>>>
>>>>>>>>>>>> I can see what Thomas is saying though. All of the checks
>>>>>>>>>>>> involve something like:
>>>>>>>>>>>>
>>>>>>>>>>>> total += some_value;
>>>>>>>>>>>> if (total < some_value) {
>>>>>>>>>>>> // We must have overflowed
>>>>>>>>>>>> vm_exit(...);
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> So a function in CollectedHeap (the base class of
>>>>>>>>>>>> SharedHeap and ParallelScavengeHeap) might make sense. For
>>>>>>>>>>>> example:
>>>>>>>>>>>>
>>>>>>>>>>>> total_reserved = 0;
>>>>>>>>>>>> total_reserved = add_and_verify_no_overflow(total_reserved,
>>>>>>>>>>>> max_heap_size);
>>>>>>>>>>>> total_reserved = add_and_verify_no_overflow(total_reserved,
>>>>>>>>>>>> max_perm_size);
>>>>>>>>>>>>
>>>>>>>>>>>> Where the function is:
>>>>>>>>>>>>
>>>>>>>>>>>> size_t add_and_verify_no_overflow(size_t x, size_t y) {
>>>>>>>>>>>> const char* msg = "...";
>>>>>>>>>>>> x += y;
>>>>>>>>>>>> if (x < y) {
>>>>>>>>>>>> vm_exit(msg);
>>>>>>>>>>>> }
>>>>>>>>>>>> return x;
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> But I can live with your current changes.
>>>>>>>>>>>>
>>>>>>>>>>>> JohnC
>>>>>>>>>>>>
>>>>>>>>>>>> On 4/10/2013 9:52 AM, Tao Mao wrote:
>>>>>>>>>>>>> Hi Bengt,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thank you for reviewing. A new webrev is updated.
>>>>>>>>>>>>> http://cr.openjdk.java.net/~tamao/6761744/webrev.01/
>>>>>>>>>>>>>
>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>> Tao
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 4/10/13 1:54 AM, Bengt Rutisson wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Tao,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This change looks good. Thanks for adding the JTReg test,
>>>>>>>>>>>>>> it looks good!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> One minor nit:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> In src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp
>>>>>>>>>>>>>> I would suggest to store "(size_t)
>>>>>>>>>>>>>> align_size_up(pgs->max_size(), HeapRegion::GrainBytes)"
>>>>>>>>>>>>>> in a local variable rather than duplicating the calculation.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Bengt
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 4/8/13 7:22 AM, Tao Mao wrote:
>>>>>>>>>>>>>>> 6761744 Hotspot crashes if process size limit is exceeded
>>>>>>>>>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-6761744
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~tamao/6761744/webrev.00/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> changeset:
>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> jtreg testing java code is also written, checking both
>>>>>>>>>>>>>>> 32-bit and (trivially) 64-bit builds.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> testing:
>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Needs JPRT test when pushing.
>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Leonid Mesnik
>>>>>>>>> JVM SQE
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Leonid Mesnik
>>>>>>> JVM SQE
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130501/e0ddd94f/attachment.htm>
More information about the hotspot-gc-dev
mailing list