Bug 48644 - Code should never ignore throwable
Summary: Code should never ignore throwable
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: trunk
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
: 52800 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-01-30 17:20 UTC by Sebb
Modified: 2012-03-01 03:37 UTC (History)
1 user (show)



Attachments
proposed patch for bug 48644 (30.45 KB, patch)
2010-04-17 12:25 UTC, CharlotteH
Details | Diff
Sample patch to fix some of the instances (3.32 KB, patch)
2010-09-13 14:38 UTC, Sebb
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sebb 2010-01-30 17:20:32 UTC
There seem to be quite a few places where code catches Throwable and ignores it.

For example:

ant.jmx.JMXAccessorQueryTask.bindAttributes
ant.jmx.JMXAccessorTask.execute
core.StandardContext - lots of methods

In most cases, catching Exception would be enough.
Comment 1 CharlotteH 2010-04-17 12:25:26 UTC
Created attachment 25319 [details]
proposed patch for bug 48644

Have created a new class and method to deal with all instances where the code catches Throwable and ignores it. The method checks whether the Throwable is something that needs to be rethrown (such as ThreadDeath or VirtualMachineError) and rethrows it if this is the case and swallows everything else. If there are any other types of Throwable that need to be dealt with instead of swallowed then they can be added to this method.
Comment 2 Mark Thomas 2010-04-24 15:27:54 UTC
Many thanks for the patch.

This has been applied to Tomcat 7.0.x and will be included in 7.0.0 onwards.

I made one small change. Jasper (org.apache.jasper) is often used as a stand-alone module so it can't depend on any external classes. Therefore, I gave Jasper its own copy of ExceptionUtils.
Comment 3 Sebb 2010-09-13 14:28:03 UTC
There are still lots of places where Throwable is caught.

In most (all?) cases, catching Throwable is unnecessary - Exception would do just as well. Or indeed a more specific one such as IOException.

Where the code really does need to catch Throwable, then the ExceptionUtils method must be used. There are also many cases where this has not been done.
Comment 4 Sebb 2010-09-13 14:38:17 UTC
Created attachment 26023 [details]
Sample patch to fix some of the instances
Comment 5 Mark Thomas 2010-09-27 18:32:06 UTC
Fixed in 7.0.x. Will be in 7.0.3 onwards.

There are almost certainly some places where Throwable could be replaced with a more specific exception. Those can be handled as separate enhancement requests.
Comment 6 Konstantin Kolinko 2012-03-01 03:37:12 UTC
*** Bug 52800 has been marked as a duplicate of this bug. ***