OutOfMemoryError is dangerous, but why more dangerous than other exceptions?-CodePudding

Conventional wisdom, on this site and elsewhere, is to not catch java.lang.OutOfMemoryError, except maybe for cleaning up, and just let the JVM crash and be restarted. The reason given for this is that it may have left the process in an invalid state. Okay, that seems like a good reason.

But... that's not exclusive to OutOfMemoryError. Any exception may have left invalid state if it's not properly handled. Why is this considered okay:

String handleRequest(String name) {
    try {
        if (name.equals("buggy")) return buggy();
        else if (name.equals("hungry")) return hungry();
        else throw new IllegalArgumentException();
    } catch (Exception e) { // <-- not catching OutOfMemoryError
        return "Error: "   e;
    }
}

String buggy() {
     bunnyLock.lock();
     // oops! can throw ArrayIndexOutOfBoundsException due to using round instead of floor
     var res = bunnies[(int) Math.round(Math.random() * bunnies.length)];
     bunnyLock.unlock();
    return res;
}

String hungry() {
     var b = new byte[2_000_000_000]; // 2 GB because why not
     ...
}

But changing the catch to Throwable isn't? The ArrayIndexOutOfBoundsException is somehow considered more safe to catch, yet buggy is the method that leaves the state invalid, while a failed allocation in hungry is no problem.

You might say, well, you should fix buggy to never leave state invalid:

String buggy() {
     bunnyLock.lock();
     try {
         // oops! can throw ArrayIndexOutOfBoundsException due to using round instead of floor
         return bunnies[(int) Math.round(Math.random() * bunnies.length)];
     } finally {
         bunnyLock.unlock();
     }
}

And I'd agree. In fact, I agree so much that I think every piece of code everywhere should be exception-safe like that. Then you may safely assume that even an OutOfMemoryError has not left any state invalid.

So, am I missing something and is OutOfMemoryError really more dangerous?

CodePudding user response：

But... that's not exclusive to OutOfMemoryError. Any exception may have left invalid state if it's not properly handled.

Different definitions.

Normal exceptions (i.e. everything except InternalError and OOME) have well defined behaviour: Either the code (by executing the statement throw someException;), or the JVM itself (for example by executing foo.instanceMethod where foo resolves to null, causing the JVM to throw an NPE), throws an exception. That means at the point the exception is thrown, execution of that context stops and execution jumps to the relevant catch block, or if there is none, that method is aborted and the caller now gets this exception thrown, and so on.

This does not cause invalid state unless you suck at programming. Unfortunately, a ton of SO answers and tutorials suck at programming, they do this:

try {
  some code here;
} catch (IOException e) {
  e.printStackTrace();
}

... which leaves code in an invalid state: The problem is, execution continues after that catch block, which is bad. The right way to write code that doesn't want to care about some exception is instead:

try {
  some code here;
} catch (IOException e) {
  throw new RuntimeException("unhandled", e);
}

That is merely ugly, vs the e.printStackTrace() version, which is downright broken. Update your IDE's templates!

At any rate, that is what you mean here with 'undefined behaviour' - crappy code. You can say that about any code: Well, that code COULD have bugs, so, what does it do? No body knows! - yeah, okay, but take that argument to its logical conclusion and your only remaining option is to toss your computer out the window and go live with the amish.

Fortunately, the java.* classes and all well-written libraries (most commonly used libraries are well written in this sense) do not do the above - if they throw stuff, the behaviour is not 'undefined'. Generally, all operations are atomic: Either it works, or, it did absolutely nothing and instead threw an exception (vs the above crappy code which acts like it works, in the sense that the method returns normally, but there's a stack trace on your console and the method did not do what its javadoc says it should be doing).

What the community means specifically when they say that OOME causes undefined behaviour is that OOME is a special snowflake, in this regard:

OOME can occur 'within' otherwise protected execution contexts: Unlike almost any other exception, it can be thrown inside the innards of the JVM that aren't expecting to deal with exceptions, and thus, it leaves e.g. a file handle half-open and in an undefined fragile state (where interacting with that handle causes bizarre exceptions or worse, undefined behaviours), and so on.

That's the difference. If, say, writing to an OutputStream you got from Files.newOutputStream causes some IOException, then behaviour is still well defined. Nothing changed in this record. Normally, given:

try (var out = Files.newOutputStream(.. path ..)) {
  out.write(5);
  out.write(11);
}

At the .write(5) command, either [A] that returns normally at which point you know that the '5' is at the very least in the OS's core file caches (no longer in your JVM's file caches), or [B] it throws an IOException that will accurately represent the issue at hand.

Let's say that .write(5) threw some exception, you caught it, and then you want to just continue to the .write(11) line. Nothing changed: You STILL have the same setup: Either that does work (let's say the exception was: You have no write access, and in the catch of that you prompt the user to fix it and sleep the thread for a while, and the user actually sudo chmod fixes it, then depending on how the OS and java works that might mean future .write calls will now work just fine!) - or most likely it won't, but you still get exceptions 'as expected': IOExceptions that properly describe the problem.

In contrast to OOME: If .write(5) throw an OOM, then .write(11) may cause your computer to blast Beethoven's Ode to Joy from the speakers as far as you know - the system is in an unstable state and the OutputStream you have may now behave in ways that the javadoc do not cover.

Note that this isn't universal. Not all OOMEs lead to an 'unstable' system. For example, this code is perfectly fine:

byte[] bigArray;
try {
  bigArray = new byte[100000000];
} catch (OutOfMemoryError e) {
  // handle the fact that you can't make em that big
}

Essentially, if any method invoke throws OOME you need to accept that various things can now be in a state that the javadoc do not describe and whose behaviour is dependent on all sorts of weird factors (OS, JVM vendor, java version, phase of the moon...), but a few primitives, mostly the new arr[size] operation, you know what happened and can reliably respond to the OOME.

CodePudding user response：

The problem with OOME is not that allocations for large memory chunks may fail, but that such allocations allocate everything thats available. And then any other allocation in any other thread will fail.

And needing to consider that any allocation may fail while coding makes it impractical from an economic perspective. You give up a lot of high level features that java and/or your techstack and business abstractions provide and need to deal with the underlying memory resources at a lot more locations than you have to deal with file io for example.

So I agree that there can be programs that are perfectly capable of handling OOME. It's just that they are not practical from an economic perspective.

Another reason is that keeping a jvm on such a high memory usage level will kick off garbage collection very often so that the cpu is basically only busy doing gc work. Also rendering it useless from an economical perspective.

CodePudding user response：

Assuming that our entire project is full of perfect code that has 0 bugs, i.e. may only encounter exceptions from system resources and user input, and always leaves state valid when such exceptions occur, one of the differences is that in code like:

bunnyLock.lock();
try {
    doBunnyThings();
} finally {
    bunnyLock.unlock(); // assume that this requires memory, e.g. because it's a custom logging implementation of Lock
}

...it is assumed that the code that the finally block only contains "safe" code that never throws. And if it's invoked because of an ArrayIndexOutOfBoundsException inside doBunnyThings, that is a pretty safe assumption. However, if OOM is thrown by doBunnyThings, chances are higher that the finally block also fails, because memory is still low. Then the assumption is violated and all bets are off, so the resulting OOM should not be caught.

That said, I'm not quite convinced. For example, the logging behind bunnyLock.unlock() may also fail because of insufficient disk space, an IOException, yet that is not considered the end of the world. And following the perfect code guidelines, bunnyLock.unlock() should leave the lock unlocked, even if its logging failed due to an OOM.