So there I was doing a code review on a colleagues code, and ended up breaking it (SURPRISE, SURPRISE!). As I was debugging it, I discovered that the issue was actually my fault. I had accidentally created some corrupt data files that it was trying to read. My colleague's code actually handled the exception decently, so that was good. Seeing as this was a web services application that was storing its data in XML files (don't ask how we came to that decision), I decided rather than restart my local weblogic, I would just get rid of the bad files and make the call again.
As I waited for windows to lie to me about how long it was going to take to delete the files, my screen suddenly changed. I got the following:
This is where things started to get interesting. Figuring there was a minor bug in our code where we had forgotten to close a stream, I discovered code similar to the following:
//code to set up the Sax parser and the SAX listener
...
try
{
saxParser.parse(file, listener);
}
catch (IOException e)
{
throw new StorageException("Unable to read [" + file + "].", e);
}
catch (SAXException e)
{
throw new StorageException("Unable to parse [" + file + "].", e);
}
//Additional code to inspect the listener state and do stuff
...
It all looked kosher to me (we decided to wrap the exceptions from the back-end storage mechanism in our own StorageException class). Upon searching the rest of the code for a stream on the file that I missed, and finding nothing, I turned to Google. I found this article that outlines the same behavior I was experiencing. So it looks like the default SAX implementation that comes with Java 6 still has this issue. However, working around it is pretty easy, though more work is required. I simply used an InputSource to feed the SAXParser rather than the file directly. This way I could control the stream closing myself (changed code highlighted):
//code to set up the Sax parser and the SAX listener
...
InputStream stream = null;
try
{
stream = new FileInputStream(file);
saxParser.parse(new InputSource(stream), listener);
}
catch (IOException e)
{
throw new StorageException("Unable to read [" + file + "].", e);
}
catch (SAXException e)
{
throw new StorageException("Unable to parse [" + file + "].", e);
}
finally
{
if(stream != null)
{
try
{
stream.close();
}
catch (IOException ex)
{
//It is OKAY to swallow this exception here, because we are trying
//to clean up resourcesand we do not want to overwrite the original
//exception if there was one.
}
}
}
//Additional code to inspect the listener state and do stuff
...
The whole try/catch in the finally clause could have been avoided if we hadn't decided to wrap things in our own exceptions (if you are curious why we did that, ask in the comments).

No comments:
Post a Comment