Request for review: 6761744 Hotspot crashes if process size limit is exceeded
Tao Mao
tao.mao at oracle.com
Fri May 3 23:50:16 UTC 2013
Hi Vladimir,
OK, JDK7u21, Solaris, sparc, and 32-bit VM. I caught it~ (It also
affects the Linux, and/or i586-platform in a slightly different way).
But, Yay! The patch should solve your problem.
Cheers,
Tao
On 5/1/13 12:45 PM, Vladimir Kempik wrote:
>
> Hi.
>
> Are you sure it's solaris sparc?
>
> I've tried on server with more than 4g memory.
>
> Crashes only on sparc 32-bit java.
>
> Other 32-bit systems do not crash, but as you saw, it should have
> errored in your case, saying it can't reserve enough memory (like it
> happens with patch applied)
>
> Vladimir
>
> 01.05.2013, в 21:36, Tao Mao <tao.mao at oracle.com
> <mailto:tao.mao at oracle.com>> написал(а):
>
>> 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/20130503/81805ef2/attachment.htm>
More information about the hotspot-gc-dev
mailing list