[PATCH] and [BUG] ContextManager and missed recycling of request & response

[Jim Rudnicki <jd...@pacbell.net>]

I have stumbled across a case where the HttpServletResponseFacade is not
being recycled.  I have found where the ContextManager misses calling the
recycle methods(see patch below)  The problem appears to be here:

 public void service( Request req, Response res ) {
 internalService( req, res );
 // clean up
 try {
     res.finish();
   req.recycle();
   res.recycle();

If finish() throws an exception, then the recycle methods get skipped.  When
this does not happen, the usingWriter flag does not get reset.  This in turn
causes endless IllegalStateException's. when further responses attempt to
getWriter().

With the patch below, I can trap the occurence of this somewhat repeatedly.
How I did it was with a redirect to a page that would in turn throw many 404
errors because all the images where missing.  I was single stepping through
the sendError() method (wanted to throw 307's).  Once the response is sent
to the client, all the redirect GET requests immeadiately show up.  They in
turn throw 404 errors that go through the error handler of the same context.
I believe that if the redirects complete before the "parent" that is when
the recycling gets lost.  It made this happen several times, but cannot make
it happen every time.  So I cannot offer a precise diagnosis, but I believe
the patch below is pointing in the right direction.  Somebody who
understands better needs to look at this.

Jim

Index: ./src/share/org/apache/tomcat/core/ContextManager.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/core/ContextManag
er.java,v
retrieving revision 1.107
diff -r1.107 ContextManager.java
617a618
>  boolean bRecycled = false;
622c623,624
<      res.recycle();
---
>      res.recycle();
>   bRecycled = true;
625a628,652
>  /* jdr-5Aug00-This traps when the recycle is missed above.
>  I do not know whether it was expected that handleError would
>  complete the recycling. Also, is it expected that the recycle()
>  methods may throw exceptions?
>  The comment below may be the better coding.
>  */
>  finally {
>   if( !bRecycled ) {
>    System.out.println( "Caught missed recycle" );
>    req.recycle();
>    res.recycle();
>   }
>  }
>  /* It appears to me that the recycle() methods do not throw any
exceptions.
>  Is it possible that this is the correct code?:
>  try {
>      res.finish();
>  } catch( Throwable ex ) {
>      handleError( req, res, ex );
>  }
>  finally {
>   req.recycle();
>   res.recycle();
>  }
>  */