Re: [OT] Questions About Tomcat Exception Handling

[Christopher Schultz <ch...@christopherschultz.net>]

Mark,

On 1/21/15 2:34 PM, Mark Thomas wrote:
> On 21/01/2015 19:14, Felipe Ebert wrote:
>> Hello
>>
>> I'm sorry that this mail is not a development issue itself, it is just a
>> question about Tomcat' exception handling. I was checking some mails at
>> Tomcat dev list and I found two interesting and contrasting responses
>> regarding exception handling: one is from Mark Thomas and another is from
>> Remy Maucherat:
>>
>> Mark Thomas:
>> http://www.mail-archive.com/dev%40tomcat.apache.org/msg45180.html
>>
>> Remy Maucherat:
>> http://www.mail-archive.com/dev%40tomcat.apache.org/msg10280.html
> 
> The tone might be different but the response is pretty much the same: we
> don't view the issues reported as bugs.
> 
>> So, I just would like to understand why Tomcat don't care about those
>> generic catches patterns (catch Throwable or catch Exception) as Remmy said
>> in his mail, which contrasts with Mark comments. And also I would really
>> appreciate if anyone could give me some insights in the matter.
> 
> Again, I don't read the substance of our responses as being that different.
> 
> Where these patterns are used they are used with good reason. If someone
> reports one that isn't backed with a good reason we'll fix it. The
> frustrating thing from our point of view is that most of the reports we
> receive along these lines are from people who clearly have zero
> understanding of the Tomcat code and are blindly reporting something
> some static analysis tool has reported.
> 
> I'm not going to justify every single time we catch Throwable but a
> typical pattern is when calling user code that can (and often does)
> throw some random exception the only viable option when any error in
> user code needs handling (even if it is to simply ignore it) is to catch
> Throwable.
> 
>> And lastly, I also would like to hear some comments about how frequent bugs
>> are reported in Tomcat mailing list and not in the Bugzilla. I know the
>> pattern is to use Bugzilla, but I saw some comments from Dr. Justin
>> Erenkrantz (from Apache team) discussing that there are a lot of important
>> bugs fixes only reported in Tomcat mailing list.
> 
> Take a look at the change log. That will give you an idea of how many
> things get fixed that aren't in bugzilla. Whether it was originally
> reported via a mailing list isn't recorded. They will be a mix of
> mailing list, things committers spot at work and things committers spot
> while looking at the code for some other reason.

I read a post from 8 years ago from some guy who had a collection of bad
days until his whole collection of web sites went dead. Anyhow, here's
the link:
http://www.jroller.com/fate/entry/why_i_hate_tomcat

He basically defecates all over the DefaultServlet, and claims it's
representative of all of Tomcat's code. Granted, it was way back in
Tomcat 5.5 days and his form of invective is, as usual, neither
constructive nor completely coherent.

But he does have a few points, and I took a few minutes looking at the
DefaultServlet in the current trunk, and I think he made some fair
points about things that still exist.

To wit:

1. org.apache.catalina.servlets.DefaultServlet.Range.validate modifies
the value of the "end" member. That seems ... odd.

2. copyRange(InputStream, ServletOutputStream) returns IOException
instead of throwing it. There is one call to copyRange() that always
ignores this return value, even if it is a non-null IOException object.

3. executePartialPut(HttpServletRequest, Range, String) has a potential
resource leak of bufOldRevStream.


I'm not well-versed in the DefaultServlet to know if 1 and 2 are
legitimate, but they definitely smell funny. The resource issue in #3
should probably be fixed, and I'm happy to provide a fix for it.

The Coverity scan has been great at finding likely bugs and stuff... the
list is just enormous and it's tough to get through it all. I suspect #3
above is in that report, buried somewhere in the million potential
issues it has found.

-chris

Re: [OT] Questions About Tomcat Exception Handling

[Felix Schumacher <fe...@internetallee.de>]

Am 22.01.2015 um 00:51 schrieb Rémy Maucherat:
> 2015-01-21 23:47 GMT+01:00 Christopher Schultz <chris@christopherschultz.net
>> :
>> To wit:
>>
>> 1. org.apache.catalina.servlets.DefaultServlet.Range.validate modifies
>> the value of the "end" member. That seems ... odd.
>>
>> 2. copyRange(InputStream, ServletOutputStream) returns IOException
>> instead of throwing it. There is one call to copyRange() that always
>> ignores this return value, even if it is a non-null IOException object.
>>
>> 3. executePartialPut(HttpServletRequest, Range, String) has a potential
>> resource leak of bufOldRevStream.
>>
>>
>> I'm not well-versed in the DefaultServlet to know if 1 and 2 are
>> legitimate, but they definitely smell funny. The resource issue in #3
>> should probably be fixed, and I'm happy to provide a fix for it.
>>
> Yes, it would be a good idea to be careful with 1 and 2.
I came along issue 2 while looking at a resource leak identified by 
coverity scan in getReadme, which is the one method, which ignores the 
returned exception. The comments in the other methods seem to indicate, 
that a try/finally block plus a conversion to throwing the exception 
instead of returning it in copyRange would be appropriate.

Regards
  Felix
>
> I don't remember reading that article, but maybe I forgot.
>
> Rémy
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

Re: [OT] Questions About Tomcat Exception Handling

[Rémy Maucherat <re...@apache.org>]

2015-01-21 23:47 GMT+01:00 Christopher Schultz <chris@christopherschultz.net
>:

> To wit:
>
> 1. org.apache.catalina.servlets.DefaultServlet.Range.validate modifies
> the value of the "end" member. That seems ... odd.
>
> 2. copyRange(InputStream, ServletOutputStream) returns IOException
> instead of throwing it. There is one call to copyRange() that always
> ignores this return value, even if it is a non-null IOException object.
>
> 3. executePartialPut(HttpServletRequest, Range, String) has a potential
> resource leak of bufOldRevStream.
>
>
> I'm not well-versed in the DefaultServlet to know if 1 and 2 are
> legitimate, but they definitely smell funny. The resource issue in #3
> should probably be fixed, and I'm happy to provide a fix for it.
>

Yes, it would be a good idea to be careful with 1 and 2.

I don't remember reading that article, but maybe I forgot.

Rémy