JVMTI OOM handling when arrays / objects are too large

Sun Jun 14 16:22:55 PDT 2009

Martin,

I agree regarding the bug - this fix provides a consistent approach to 
all situations where an OOME is thrown. (You can consider this an 
additional review.)

My meta-question is whether OOME should have been thrown for this case: 
it somehow seems wrong to me - I would not handle this error in the same 
way I would handle a true OOM, because this is obviously a simple 
programming error - but other than throwing an unspecified 
RuntimeException I don't see a better alternative. And it's a little 
late to change this now.

My caution with the fix is that it will now post JVMTI events in 
circumstances where they were not previously posted. I doubt any real 
code will be affected by this, but it is possible that there is a test 
somewhere that may get upset by it. You know how sensitive some of our 
tests can be. ;)

Cheers,
David

Martin Buchholz said the following on 06/15/09 08:43:
> I'm far from an expert on jvmti or hotspot's error handling, but...
> 
> If I've asked for a command to be run on OOME,
> having it not run feels very much like a bug.
> I agree that OOME might not perfectly match the situation,
> but is there a better course of action?
> I don't see any reasonable alternative to throwing OOME.
> 
> Martin
> 
> On Sun, Jun 14, 2009 at 15:25, David Holmes - Sun Microsystems 
> <David.Holmes at sun.com <mailto:David.Holmes at sun.com>> wrote:
> 
>     Martin, Jeremy,
> 
>     This change has been bugging me. I guess what I don't like is not
>     the "fix" but the fact that we report OutOfMemoryError for this
>     situation in the first place! We are not out-of-memory! We've been
>     asked to allocate something that is impossible to allocate given the
>     physical constraints of the memory system. The heap could actually
>     be nearly empty! If I were executing a OOM handler using the
>     "onError" mechanism I'm not sure I'd want it to run for this case.
> 
>     This fix makes this usage consistent with all the other OOM
>     situations, but we post JVMTI resource exhausted events when the
>     resource need not be exhausted at all! I'm not sure that is the
>     right thing to do. Even assuming it is the right thing to do, this
>     may impact some tests as it now generates additional JVMTI events.
> 
>     David Holmes
> 
>     Martin Buchholz said the following on 06/15/09 05:37:
> 
>         I've polished the changes in preparation for committing.
>         I'll commit once I have reviewer approval.
>         You'll need to let me know which forest to commit to.
> 
>         Test webrev is now at:
>         http://cr.openjdk.java.net/~martin/jvmti-oom-test/
>         <http://cr.openjdk.java.net/%7Emartin/jvmti-oom-test/>
> 
>         Now has more tests and filled-in @bug line.
> 
>         Hotspot fix webrev is at:
>         http://cr.openjdk.java.net/~martin/jvmti-oom/
>         <http://cr.openjdk.java.net/%7Emartin/jvmti-oom/>
> 
>         I've hacked on my private copy of webrev to make the output more
>         suitable for
>         external contributors.
> 
>         I think it's time to again beg the hotspot integrators
>         to be sure to run the java/lang and java/util/concurrent tests
>         from the jdk repo before committing changes to MASTER.
> 
>         Martin
> 
>         On Sat, Jun 13, 2009 at 10:18, Tim Bell <Tim.Bell at sun.com
>         <mailto:Tim.Bell at sun.com> <mailto:Tim.Bell at sun.com
>         <mailto:Tim.Bell at sun.com>>> wrote:
> 
>            Martin Buchholz wrote:
> 
>                I've called my own bluff and implemented a test case for this
>                http://cr.openjdk.java.net/~martin/jvmti-oom/
>         <http://cr.openjdk.java.net/%7Emartin/jvmti-oom/>
>                <http://cr.openjdk.java.net/%7Emartin/jvmti-oom/>
> 
> 
>                Jeremy's original fix is in this hotspot webrev:
>                http://cr.openjdk.java.net/~martin/jvmti-oom-hotspot/
>         <http://cr.openjdk.java.net/%7Emartin/jvmti-oom-hotspot/>
>                <http://cr.openjdk.java.net/%7Emartin/jvmti-oom-hotspot/>
> 
> 
>                Sun folks (Tim?), please take up the process issues:
>                - please review test and fix
>                - file one (or two?) "real" bugs or
> 
> 
>            For the HotSpot VM side:
> 
>                6850957 hotspot/jvmti JVMTI OOM handling when arrays /
>         objects
>                are too large
> 
> 
>            For the test case:
> 
>                6850958 java/classes_lang JVMTI OOM handling when arrays /
>                objects are too large
> 
> 
> 
> 
>                It's non-traditional to have fixes cross the hotspot/jdk
>         barrier,
>                but this was the easiest way to write a test case.
> 
> 
>            This happens most often in the Serviceability area, for
>         example when
>            fixes hit JVM TI and JDWP code.  You have another good
>         example, where
>            the most natural test case fits in
>         JDK/test/java/lang/ProcessBuilder
> 
>            The parent bugzilla report is:
> 
> 
>             https://bugs.openjdk.java.net/show_bug.cgi?id=100067
> 
>            I filed two internal bug reports that should be visible on
>            bugs.sun.com <http://bugs.sun.com> <http://bugs.sun.com>
> 
>            in a few working days.  Using URL surgery, I predict the URLs
>         will be:
> 
>             http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6850957
>             http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6850958
> 
>            Later today I will set up a forest, apply the patches from the
>            webrevs, and send it through JPRT.  I want to see the HotSpot
>            test results, as well as the new ProcessBuilder/Basic.java
> 
>            Tim
> 
> 
> 