Request for review: 6761744 Hotspot crashes if process size limit is exceeded

[Thomas Schatzl]

Hi,

On Wed, 2013-04-10 at 11:52 -0400, Coleen Phillimore wrote:
> On 4/10/2013 11:10 AM, Thomas Schatzl wrote:
> > Hi,
> >
> > On Wed, 2013-04-10 at 16:53 +0200, Thomas Schatzl wrote:
> >> Hi,
> >>
> >> On Sun, 2013-04-07 at 22:22 -0700, 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.
> >> I think this check should be done once in Universe::reserve_heap() where
> >> the actual reservation occurs, just before that if possible instead of
> >> special handling for every collector.
> >>
> >> Additionally, Universe::reserve_heap() also does final alignment before
> >> reserving the heap which might influence the actually reserved heap
> >> size.
> > Okay, I just saw that I was looking at hsx25, sorry. In hsx24 there is
> > no Universe::reserve_heap() which all collectors call to actually
> > reserve the heap.
> >
> > Not sure if it is part of this CR to unify this handling this for hsx24
> > into a single method similar to hsx25.
> 
> I don't think you can do this because one of the version does extra junk 
> for class data sharing.

I wasn't exactly thinking of backporting the fix verbatim but doing
something similar - but probably it is beyond the scope of this CR.

I am leaving this up to Tao and the others to decide.

@Coleen: I was asking myselves when comparing with the code for
calculating the maximum heap for ergonomically enabling compressed oops:
isn't the limit for heap+perm actually 2^bitsize - 1 OS page (the null
page excluded), do you know?

> 
> Coleen
> (lurking on hotspot-gc)
> >
> > However, it may be interesting anyway to refactor the actual overflow
> > checking, i.e. some method that gets passed java heap size (I think
> > there is already the invariant that young + old gen size is always less
> > than max heap size, i.e. less than 4G on 32 bit), perm gen size and
> > alignment and is called by all three CollectedHeap implementations.
> >

As mentioned I still believe it would be nice to have the checking code
extracted into a single method that is shared and used by all of the
heap implementations.

Replicating the same (string) constant and basically the same checking
three times in the code seems undesirable.

Thomas