Reputation: 202
In Rational Application Developer(RAD based on eclipse), under software analyzer, I see a code review comment (under Performance =>Memory section) saying "Avoid the throw statement inside of finally".
How does defining throw inside a finally block affect performance?
Here is code snippet, we have already suggested to change code to log exception trace and do not throw exception,
} finally {
if (bufferedReader != null) {
try {
bufferedReader.close();
} catch (final IOException ex) {
throw ex;
}
}
}
I was just wondering how this could affect memory and performance?
Upvotes: 13
Views: 4374
Reputation: 96394
This is not a performance issue. It is a correctness issue. (Marko Topolnik's comment that the warning is misclassified seems correct to me, the only way I can see a performance angle to this is that if an exception thrown in the try block gets masked, the effort spent creating it and its stack trace was wasted. But this is a long way from being the biggest problem.)
Two reasons not to throw an exception in a finally block:
Letting an exception get thrown from the finally block can mask any exception thrown in the try block, so you lose the original, useful exception, leaving no clue in the logs as to what actually went wrong.
When your normal flow of control gets interrupted for an exception thrown on close, you may be letting some transitory I/O glitch (that you don't have any control over and which didn't affect your business logic) prevent some piece of useful work from getting accomplished (for instance, it might cause the current transaction to be rolled back). This may depend on what kind of resource is involved; maybe there is some case where you really may want to fail the whole thing if the close doesn't happen cleanly, but for a lot of common cases (like JDBC) there is no good reason to care.
Using try-with-resources successfully excludes the possibility of exception-masking. However, if the try logic completes without an exception, it lets anything thrown on close get propagated. Since it's a language addition Oracle has to take the most conservative approach, just be aware what it's doing when you use it.
Upvotes: 3
Reputation: 269697
An exception thrown from the finally
block will replace any exception that was thrown from the try
, and information about the real problem is likely to be lost.
Since the try-finally
block is allowed to throw an IOException
in this case, here's a better way to write it:
try (BufferedReader bufferedReader = Files.newBufferedReader(Paths.get("file.txt"))) {
/* Work with `bufferedReader` */
}
This automatically closes the reader when the block exits, and handles any resulting exceptions nicely, even if the code inside the try
block throws its own exception first, using the "suppression" mechanism of Throwable
.
If the try
block completes without exception, the result will be the result of closing the resource (exception or not). If the try
block throws an exception, that will be the exceptional result. But if the close()
method raises an exception too, it will be added to the try
block's exception as a "suppressed" exception. You can interrogate it programmatically, and when the stack trace is printed, the suppressed exception will be displayed, much like the "caused by" exceptions you may be more familiar with.
And, you can try-with-multiple resources; these will all be closed, and multiple closure exceptions can be suppressed.
This assumes you're using file I/O, but the same "try-with-resources" structure will work on anything that implements AutoCloseable
(streams, SQL objects, etc.).
Upvotes: 6
Reputation: 5103
Ideal solution:
try (BufferedReader bufferedReader = ...) {
//do stuff
}
But perhaps you're in java 1.6:
BufferedReader bufferedReader = null;
try{
bufferedReader = ...;
//do stuff
} finally {
if (bufferedReader != null) {
try {
bufferedReader.close();
} catch (final IOException ex) {
logger.error("Problem while cleaning up.", ex);
}
}
}
Upvotes: 1
Reputation: 352
In the code snippet given, there is no point to the throw statement. The exception would have already been thrown by the bufferedReader.close() method. The catch block should just handle it, rather than throwing another exception. In fact, while catching a thrown exception is certainly valid in a finally block, I cannot really think of a valid reason to throw an exception in a finally block.
As to performance degradation, from an anecdotal standpoint, rethrowing a caught exception is clearly less efficient than just handling it.
As to a specific instance where this would be detrimental, something like this comes to mind off the top of my head. If you had another method performing cleanup in your finally block, such as FileOutputStream.close(),and the first one threw an error, the second would never run, as the throw statement ends the processing of the block. At that point you would be leaking resources.
So, to sum it up, try/catch are fine in a finally block, but the use of the throw statement should be avoided for the sake of efficiency as well as unintended consequences (memory leak).
Upvotes: 0
Reputation: 106
In finally block you can call another method passing object B to clean up. And you can have try catch finally blocks in that method.
Its good if you have clean up methods for each object A & B separately, in which you handle try catch finally blocks. You can call both the methods for cleaning up objects. Exceptions from each object clean up are independently handled.
Upvotes: 0