What's the best way to handle this "try - error - clean -retry" case?

Question

I am trying to load an image in memory but might have memory issues since i have some other images loaded. These images have a "visible" field that dictates whether they are visible or not. Regardless of the visibility i keep them in memory for fast loading (when they become visible again).

But since i have many of them in memory i want to try to load a new image and if i run into memory issues, release the non-visible images and try again. Now so far i am using this rather ugly (and wrong for some reason, i am sure) piece of code :

try {
    image = GraphicsUtilities.loadImage(filePath);
} catch (OutOfMemoryError e) {
    removeHiddenImageReferences();
    try {
        image = GraphicsUtilities.loadImage(filePath);
    } catch (OutOfMemoryError ee) {
        ee.printStackTrace();
        JOptionPane.showMessageDialog(parent,
            "There is not enought memory to load this image",
            "Not enough memory", JOptionPane.WARNING_MESSAGE);
    }
}

My question is, how should i handle this kind of case? I feel that catching an exception and re catching an exception inside the catch clause is bad.

Answer 1

I don't see this as particularly bad style - the whole point of the "catch" clause is to handle the cause of the exception. In this case, you choose to handle running out of memory by freeing some, then allowing one further retry before propagating an error. Seems perfectly reasonable - you could rewrite it in any number of syntactically contorted ways to avoid having code inside the catch clause, but why bother? This clearly expresses your intent.

However, one design change that might make sense would be to rethrow the exception in the second case (or, just don't catch it), and have a more global exception handler to deal with full memory. Even this is debatable - if you don't really expect any other OutOfMemory situations, then handling it close to the scenario that (nearly) always causes it is reasonable.

Answer 2

Introduce a while loop so your retry logic is tied to a condition, not just the exception being thrown.

int tryCount = 2;
while (tryCount > 0) {
  try {
    image = GraphicsUtilities.loadImage(filePath);
  } catch (OutOfMemoryError e) {
    removeHiddenImageReferences();
    tryCount--;
  }
}
if(tryCount <= 0) {
        JOptionPane.showMessageDialog(parent,
            "There is not enought memory to load this image",
            "Not enough memory", JOptionPane.WARNING_MESSAGE);
}

This example misses the stacktrace, but at this point it's not that important because you're out of memory (just include an identifier in the MessageDialog or log where this check is being made).

Answer 3

One option would be to break it out into 2 methods, you call LoadImage which calls TryLoadImage internally.

public void TryLoadImage(string filePath){
     bool b = false;

     try{
          image = GraphicsUtilities.loadImage(filePath);
          bool b = true;
     }
     catch(OutOfMemoryError){
          // Log notfication of file the was too big perhaps? so in the future you could
          // optimize this?  
     }
}

public void LoadImage(string filePath, bool clearReferences){

      if(!TryLoadImage(string filePath))    
      {
            removeHiddenImageReferences();
            if(!TryLoadImage(string filePath)){
                   // Log error
                   JOptionPane.showMessageDialog ....
            }
      }
}

Answer 4

This is a tough one, as OutOfMemoryErrors are not specific to individual Threads. This means that though in the current Thread you are loading an image that takes the bulk of your process's memory, another Thread might be running the process that actually triggers the OutOfMemoryError. Adam Wright's idea of a more global OOME manager would be something to consider if you need to handle this case in-process.

edit:

This article talks about setting up a memory warning system, so that listeners can be alerted before you trigger the OOME. This allows you to take preventive measures and save your system from the debilitating OOME. Having read your comment, I am posting the link here for posterity.

Answer 5

Have you considered using SoftReference? While it wouldn't necessarily change your code here, it would make the situation more scalable if your application is doing other things and you would be willing to jettison these images if it ran out of memory somewhere else.

Answer 6

You could call

removeHiddenImageReferences();

first. Then you only need one try/catch. There's no use optimizing, unless you KNOW removeHiddenImageReferences() is horribly slow (like if you're working with millions of objects). Usually you can't do too much about running out of memory anyways.